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

[svelte-check] cannot force color output #2662

Open
marekdedic opened this issue Jan 13, 2025 · 5 comments · May be fixed by #2678
Open

[svelte-check] cannot force color output #2662

marekdedic opened this issue Jan 13, 2025 · 5 comments · May be fixed by #2678

Comments

@marekdedic
Copy link

marekdedic commented Jan 13, 2025

Describe the bug

Hi, I am running svelte-check with npm-run-all with output aggregation. This normally removes color support (as it probably should), but I would like to force svelte-check to output colors...

Reproduction

{
  "scripts": {
    "lint:svelte-check": "svelte-check",
    "lint": "run-p --aggregate-output lint:*"
  }
}

Expected behaviour

I'd expect the following to be the same:

npm run lint:svelte-check:

image

npm run lint:

image

System Info

  • OS: Ubuntu 24.04
  • Shell: zsh

Which package is the issue about?

svelte-check

Additional Information, eg. Screenshots

I also tried FORCE_COLOR=1, but that gives an in-between result with some colors. I think it's great to support thing like NO_COLOR, but my suggestion would be to add --color that enables colored output as well.

FORCE_COLOR=1 npm run lint:

image

@marekdedic marekdedic added the bug Something isn't working label Jan 13, 2025
@jasonlyu123
Copy link
Member

jasonlyu123 commented Jan 15, 2025

The colouring of the error message for the Sass compiler comes directly from the Sass compiler. So it's out of svelte-check's control. I did some digging and it seems like sass only supports --color while sass-embbed support the FORCE_COLOR variable. Can you check if you were using sass and check if it works if you install sass-embbed?

@jasonlyu123 jasonlyu123 added upstream awaiting submitter and removed bug Something isn't working labels Jan 15, 2025
@marekdedic
Copy link
Author

Thanks for the response, sass-embedded doesn't work for me, the results is the same.

it seems like sass only supports --color

All the more reason to add --color to svelte-check and then pass it down :D

@jasonlyu123
Copy link
Member

jasonlyu123 commented Jan 17, 2025

Hmm. It works for me in an empty Sveltekit project. Maybe it has something to do with your preprocess setup. Can you provide a reproduction repository? Maybe the Sass compiler is executed in another child process or something. But if that is the case, we can't pass down the --color argument or the FORCE_COLOR variable since the child process isn't created by svelte-check but from your config.

@marekdedic
Copy link
Author

Sorry for the slow response, but to summarize:

  • I am not really that interested in fixing a one-off problem with sass-embedded, which I don't regularly use (I only used it to test your hypothesis)
  • Independently of that, I think it would be great to support color output with whatever downstream tools are used. I'd be willing to try making a PR if you think that's something you want to support as well.
    Thanks!

@jasonlyu123
Copy link
Member

I mentioned sass-embedded because it is the modern alternative to sass. So if sass-embedded could work then the problem is solved. The reason why I asked for a reproduction is that if the problem is your preprocess config runs sass in another thread/child process. Then there is nothing we can do. Supporting --color doesn't help because we can't control the child process to pass down the flag.

I think it's ok to have the --color flag since it is also supported by the coloring libary we used. Just wanted to make sure it can work for you. If you're interested in the PR you can test it yourself. Then I don't need the reproduction anymore. It should just be adding the argument to svelte-check's CLI options so that sade does errors with unknown options.

@marekdedic marekdedic linked a pull request Feb 3, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants