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

Prettier format url path with number incorrectly #5654

Closed
jasonxia23 opened this issue Dec 18, 2018 · 12 comments · Fixed by #7592
Closed

Prettier format url path with number incorrectly #5654

jasonxia23 opened this issue Dec 18, 2018 · 12 comments · Fixed by #7592
Labels
lang:css/scss/less Issues affecting CSS, Less or SCSS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! type:bug Issues identifying ugly output, or a defect in the program

Comments

@jasonxia23
Copy link

jasonxia23 commented Dec 18, 2018

Prettier 1.15.3
Playground link

--parser css

Input:

body {
  background: url(http://123.example.com);
}

Output:

body {
  background: url(http://123example.com);
}

Expected behavior:

@ikatyang
Copy link
Member

As a workaround, you could use quoted url for now:

Prettier 1.15.3
Playground link

--parser css

Input:

body {
  background: url('http://123.example.com');
}

Output:

body {
  background: url("http://123.example.com");
}

@ikatyang ikatyang added type:bug Issues identifying ugly output, or a defect in the program priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! lang:css/scss/less Issues affecting CSS, Less or SCSS labels Dec 18, 2018
@jasonxia23
Copy link
Author

Maybe I can help to fix this. Could you give me some advise.

@lydell
Copy link
Member

lydell commented Dec 19, 2018

As far as I remember, the issue is that the parser doesn’t treat the whole url(...) thing as one token. Instead, it thinks it’s a general function and splits it into url, (, http, :, /, /, ..., ). So all sorts of weird stuff can happen.

@jasonxia23
Copy link
Author

I have spent some time on parser-postcss.js and printer-postcss.js. If I am not making a mistake, tokenizing happens in parser-postcss.js, right? Will it lead to other errors if I just take url(...) as one token. @ikatyang

@ikatyang
Copy link
Member

cc @evilebottnawi

@alexander-akait
Copy link
Member

alexander-akait commented Dec 27, 2018

Bug in parser, postcss-values-parser broken with url (when it is not "string") #5654 (comment) and i am banned in postcss-values-parser and i can't send a PR (owner don't love me 😕 ). Maybe we need switch on postcss-value-parser (we use this in stylelint, webpack loaders/plugins and other repos) and it is works as expected and no problems, but it is require rewrite many logic. I am planned this on next year (January/February).

@jasonxia23
Copy link
Author

Seems to be complicated. Would be glad if I can do some help.

@alexander-akait
Copy link
Member

alexander-akait commented Dec 28, 2018

@jasonxia23 I can't, i am banned, sorry, this guys violated Code of Conduct many times and i don't want use his code and work with him also, so we will switch on better and right solution like postcss-value-parser in next year as i said above, anyway feel free to fix this problem on prettier side, you can check isValidUrl here https://github.com/prettier/prettier/blob/master/src/language-css/parser-postcss.js#L31 (you can get logic ) and return value-url node and implement how we should print value-url (maybe better name)

@jasonxia23
Copy link
Author

@evilebottnawi Maybe parser should not parse 123.example.com into 123., example, . and com, which cause the issue directly.

@lydell
Copy link
Member

lydell commented Dec 30, 2018

Yes, unquoted urls are a special case in the tokenization: https://www.w3.org/TR/css-syntax-3/#url-token-diagram

@shellscape
Copy link
Contributor

@lydell @jasonxia23 evilbotnawi is banned from my repos because he has violated the code of conduct in my repos repeatedly, is generally a toxic individual to work with, and (as evidenced by his comments here) likes to repeat lies about me on various social media sites.

I'm more than happy to help to resolve this bug, but if the intention of the project is to just use a different module soon anyways, I'll keep my focus on the new version rewrite instead. I won't be offended at all I promise, I just have limited time because we'll be welcoming a child soon 😄

@shellscape
Copy link
Contributor

I have published v3.0.0-beta.2 of postcss-values-parser. There are a lot of breaking changes as the entire parser was rewritten, but this should resolve most of the issues in Prettier associated with the module. If you all would like assistance upgrading, please let me know, I'm happy to help.

@thorn0 thorn0 linked a pull request Mar 31, 2020 that will close this issue
3 tasks
thorn0 added a commit to mattiacci/prettier that referenced this issue May 29, 2020
@github-actions github-actions bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Aug 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:css/scss/less Issues affecting CSS, Less or SCSS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. priority:high Code is printed in a way that alters the AST, breaks syntax, or is a significant regression. Urgent! type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants