Skip to content

Conversation

@gucong3000
Copy link
Owner

@gucong3000 gucong3000 commented Jun 12, 2018

  • Infer HTML and XML by source for unknown file extension
  • return css for other unknown file extension

stylelint/stylelint#3328 (comment)

@codecov
Copy link

codecov bot commented Jun 12, 2018

Codecov Report

Merging #26 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #26   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          11     13    +2     
  Lines         250    258    +8     
=====================================
+ Hits          250    258    +8
Impacted Files Coverage Δ
index.js 100% <100%> (ø) ⬆️
parse.js 100% <100%> (ø) ⬆️
is-html.js 100% <100%> (ø)
languages.js 100% <100%> (ø)
parser.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eacae3e...5f2b1a9. Read the comment docs.

@gucong3000
Copy link
Owner Author

gucong3000 commented Jun 12, 2018

@shinnn
I used a new scheme to determine whether the file is XML.
I use regular expressions to determine if the source of the file is XML.
However, it is not valid for empty files.
I suggest that before applying the XML file to stylelint, determine if it contains <xsl:stylesheet>

@shinnn
Copy link
Collaborator

shinnn commented Jun 12, 2018

What do you mean by "empty files"?

At least the following code doesn't throw any errors.

const postcss = require('postcss');

const result = postcss().process('', {
  syntax: require('postcss-syntax'),
  from: '/path/to/example.xml'
});

@gucong3000 gucong3000 requested a review from shinnn June 12, 2018 07:51
@gucong3000
Copy link
Owner Author

What do you mean by "empty files"?

Empty string source.

*.xml is the only exception because this is the case in the regular, but the XML format supported by vscode has many other extensions, just like: *.svg, *.atom *.mxml, *.xhvml, *xvml, *xvm, I can't write all of them into this regular, because there are so many.

const postcss = require('postcss');

const result = postcss().process('', {
  syntax: require('postcss-syntax'),
  // empty source `*.svg` file will berak postcss-syntax
  from: '/path/to/example.svg'
});

@gucong3000
Copy link
Owner Author

image

@shinnn
Copy link
Collaborator

shinnn commented Jun 14, 2018

Post #26 (comment) first. #26 (comment) was extremely confusing to me.

I can't write all of them into this regular, because there are so many.

Could you at least support all the XML-related extensions you know currently?

@gucong3000
Copy link
Owner Author

In addition to XSL, other XML is actually not supported by us because they are not style files.
I'm very puzzled, why should we apply a style-independent file to a tool that handles styles?

I found that there are about 100+ extensions for XML format, but in fact we only support one (XSL).

Copy link
Collaborator

@shinnn shinnn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

@gucong3000 gucong3000 force-pushed the xml branch 2 times, most recently from d47c713 to 9a9deb0 Compare June 14, 2018 06:20
@shinnn
Copy link
Collaborator

shinnn commented Jun 18, 2018

@gucong3000 What is blocking this from being merged?

@gucong3000
Copy link
Owner Author

What is blocking this from being merged?

Some integration test runs failed:

packages version build
postcss-syntax NPM version Travis
postcss-html NPM version Travis
postcss-markdown NPM version Travis
postcss-jsx NPM version Travis
postcss-styled NPM version Travis

@gucong3000
Copy link
Owner Author

Replace with #28

@gucong3000 gucong3000 closed this Jun 20, 2018
@gucong3000 gucong3000 deleted the xml branch June 20, 2018 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants