shellscape / postcss-less

PostCSS Syntax for parsing LESS
MIT License
122 stars 39 forks source link

incorrect result on parsing at-rules in less #155

Open ssivanatarajan opened 3 years ago

ssivanatarajan commented 3 years ago

Raised an issue in stylelint https://github.com/stylelint/stylelint/issues/5210

LESS

@border-top:none !important;

while parsing the above code, getting property as "@borde-top:none" instead of "@border-top" . if there is no space between ":" and property, value getting incorrect results.

Errors

Expected Behavior

while parsing the property should be "@borde-top".

Actual Behavior

while parsing the property is "@borde-top:none".

How can we reproduce the behavior?

Parse the given sample code.

shellscape commented 3 years ago

Pull requests are welcome!

ssivanatarajan commented 3 years ago

Seems it was due to the regexp used for tokenize in postcss library. will report the issue in postcss

ssivanatarajan commented 3 years ago

@shellscape can u check the comment https://github.com/postcss/postcss/issues/1548#issuecomment-809388012 @Semigradsky suggesting using own tokenizer in postcss-less. is it possible?

shellscape commented 3 years ago

yeah @ai has spoken with me before about inheriting the tokenizer from PostCSS. It's a lot to maintain, and the PostCSS tokenizer works for 99% of cases. We customize this project where necessary and where discrepancies exist between CSS and LESS. I have no plans to change how this project works, but I will accept bug fixes and modifications.

ai commented 3 years ago

@ssivanatarajan I recommend forking this parser, copy the PostCSS tokenizer to the fork, fix it for the Less case. I will promote the fork.

Unfortunately, @shellscape’s way saves only his time, but increases a lot number of bugs and reduces ecosystem stability. We already had a few problems in ecosystems because of the private API usage in postcss-less.

shellscape commented 3 years ago

not to sound selfish, but that is mostly true. it is a major time saver for me. if a fork happens, I will fully support it

ssivanatarajan commented 3 years ago

@ssivanatarajan I recommend forking this parser, copy the PostCSS tokenizer to the fork, fix it for the Less case. I will promote the fork.

Unfortunately, @shellscape’s way saves only his time, but increases a lot number of bugs and reduces ecosystem stability. We already had a few problems in ecosystems because of the private API usage in postcss-less.

@ai this postcss-less using postcss library for both parsing and tokenizing. So do i need to create a separate parser and tokenizer based on postcss?

ssivanatarajan commented 3 years ago

@shellscape seems issue was due to the regexp used for identifying the end position of at-rule (RE_AT_END : /[ \n\t\r\f{}()'"\;/[]#]/g) , if i add ":" to this regexp , this issue is not coming. i am not sure whether it will affect any other cases? can u please check?

shellscape commented 3 years ago

@ssivanatarajan please fork the repo to make changes and run tests. You'll probably need to add a test for your reported case. Then if all tests pass, you'll know the change is good.

ssivanatarajan commented 3 years ago

@shellscape these regexp changes need to be done in postcss library.

This postcss-less library using postcss for parsing and tokenizing less, so shall I fork postcss-less project and implement both parsing and tokenizing or just change that regexp in postcss fork?

ai commented 3 years ago

Yeap, fork postcss-less. Copy tokenizer from postcss and apply your fixes.

ssivanatarajan commented 3 years ago

@ai I am a bit confused here, I need to copy both parser and tokenizer from postcss right? because this postcss-less using postcss for parsing less code also. Am I missed anything?

ai commented 3 years ago

Yeap, it will be better to copy parser too.

ssivanatarajan commented 3 years ago

ok thanks.

AkonXI commented 2 years ago

I got a similar problem when I use prettier to format code, #issue 11782 in prettier/prettier