Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect result on parsing at-rules in less #155

Open
ssivanatarajan opened this issue Mar 29, 2021 · 15 comments
Open

incorrect result on parsing at-rules in less #155

ssivanatarajan opened this issue Mar 29, 2021 · 15 comments

Comments

@ssivanatarajan
Copy link

ssivanatarajan commented Mar 29, 2021

  • Node Version:12.16.1
  • NPM Version:6.13.4
  • postcss Version:7.0.35
  • postcss-less Version:3.1.4

Raised an issue in stylelint stylelint/stylelint#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
Copy link
Owner

Pull requests are welcome!

@ssivanatarajan
Copy link
Author

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

@ssivanatarajan
Copy link
Author

@shellscape can u check the comment postcss/postcss#1548 (comment)
@Semigradsky suggesting using own tokenizer in postcss-less. is it possible?

@shellscape
Copy link
Owner

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
Copy link

ai commented Mar 29, 2021

@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
Copy link
Owner

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
Copy link
Author

@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
Copy link
Author

@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
Copy link
Owner

@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
Copy link
Author

@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
Copy link

ai commented Mar 31, 2021

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

@ssivanatarajan
Copy link
Author

@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
Copy link

ai commented Mar 31, 2021

Yeap, it will be better to copy parser too.

@ssivanatarajan
Copy link
Author

ok thanks.

@AkonXI
Copy link

AkonXI commented Nov 16, 2021

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

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

No branches or pull requests

4 participants