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

Fix use of unicode symbols in formatters for native Windows cmd #5741

Closed
XhmikosR opened this issue Nov 25, 2021 · 7 comments · Fixed by #7981
Closed

Fix use of unicode symbols in formatters for native Windows cmd #5741

XhmikosR opened this issue Nov 25, 2021 · 7 comments · Fixed by #7981
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@XhmikosR
Copy link
Member

XhmikosR commented Nov 25, 2021

What steps are needed to reproduce the bug?

  1. Run stylelint on Windows 10 native cmd (haven't tried 11)

What Stylelint configuration is needed to reproduce the bug?

Doesn't matter as long as you get an error or one of the symbols here:

const symbols = {
info: blue('ℹ'),
warning: yellow('⚠'),
error: red('✖'),
success: green('✔'),
};

How did you run Stylelint?

stylelint "**/*.{css,scss}" --cache --cache-location .cache/.stylelintcache --rd

Which version of Stylelint are you using?

14.1.0

What did you expect to happen?

The characters should show properly

What actually happened?

image

Does the bug relate to non-standard syntax?

No response

Proposal to fix the bug

No response

@ybiquitous
Copy link
Member

@XhmikosR Thank you for creating this issue. This limitation has been introduced by the log-symbols package refactoring (PR #5562).

A solution is adding the fallback symbols like log-symbols does:

https://github.com/sindresorhus/log-symbols/blob/6aea9853e1fb85a437f868d0ef400ce122c08c1d/index.js#L11-L16

but the is-unicode-supported logic also seems necessary for adding the fallback. (is-unicode-supported is pure ESM... 😓 )

Do you have any thoughts?

@ybiquitous ybiquitous added the status: needs discussion triage needs further discussion label Nov 26, 2021
@XhmikosR
Copy link
Member Author

Not sure TBH. I'd use an existent package, personally, even if it's an older version.

That being said, it seems someone wrongfully ignored the failures on Windows 😛: https://github.com/stylelint/stylelint/blob/main/lib/formatters/__tests__/prepareFormatterOutput.js

These failures there were valid and it's the same reason for this issue.

@ybiquitous
Copy link
Member

I'm not sure about Windows, but how many people use Stylint on the native cmd (not supporting Unicode)? Is it worth adding dependencies or logic?

Also, it seems good to fix lib/formatters/__tests__/prepareFormatterOutput.js if we could avoid CI failures for Windows.

@XhmikosR
Copy link
Member Author

There would be no failures if the issue I reported didn't exist.

When something is broken, we shouldn't work around it in tests, but try to fix it instead.

@jeddy3 jeddy3 changed the title Unicode Symbols don't work on native Windows cmd Fix use of unicode symbols in formatters for native Windows cmd Feb 10, 2022
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule and removed status: needs discussion triage needs further discussion labels Feb 10, 2022
Copy link
Contributor

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.

@github-actions github-actions bot added status: ask to implement ask before implementing as may no longer be relevant and removed status: ready to implement is ready to be worked on by someone labels Jan 22, 2024
@Mouvedia
Copy link
Member

@ybiquitous
Copy link
Member

The next Stylelint version can use the pure package, so it makes sense for me to avoid vendoring if people don't have any trouble with this issue so much at this time.

@ybiquitous ybiquitous added status: wip is being worked on by someone and removed status: ask to implement ask before implementing as may no longer be relevant labels Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

4 participants