chalk / chalk-cli

Terminal string styling done right
MIT License
279 stars 16 forks source link

Using in non-tty environment giving errors #11

Closed ryancat closed 3 years ago

ryancat commented 5 years ago

I am using this cli tool in lerna, which is not using tty channel to spawn child process for running npm scripts. In that case, the chalk command always faile with error code 1

Invalid style: THE_STRING_I_WANT_TO_LOG

Further investigate indicates if not using tty, this package is using getStdin from get-stdin to get the input, and trying to process all input as style. This is wrong as the data I want to print is also part of styles array, which is not a preset style, hence the error.

Run this in any non-tty process and you can reproduce the issue.

Qix- commented 5 years ago

Code please. Your description is incredibly vague. I have no idea how to begin to reproduce this.

ryancat commented 5 years ago

You can use this repo to reproduce this issue. https://github.com/ryancat/reproduce-chalk-cli-11

Nantris commented 3 years ago

We're seeing the same in PowerShell if we invoke chalk via a script runner, like yarn/npm/gulp with a command like chalk bgRed bold 'DANGER ZONE'

But running that command directly in PowerShell does work. Any thoughts?

Nantris commented 3 years ago

PowerShell 3 or 7, both have the same behavior.

Nantris commented 3 years ago

Using the templating format instead unfortunately is not a workaround.

chalk -t '{red DANGER ZONE}'

leads to:

The --template option only accepts 1 argument
error Command failed with exit code 1.

Again, this is only an issue when running with yarn or the like, but that's a rather large issue I feel. Unfortunately I have no idea how to contribute to resolution besides reporting my findings.

I confirmed that it's not due to yarn calling some other version of chalk by running a test like yarn test-chalk where test-chalk runs which chalk.

Nantris commented 3 years ago

Just discovered this problem only occurs if the string to print contains spaces.

@Qix-, @sindresorhus, any thoughts?

Edit: Printing strings with spaces works with escaped double quotes.

ashleykolodziej commented 3 years ago

I am having this issue as of today using GitHub Actions. Here's what my run looks like:

Screen Shot 2021-09-09 at 8 02 24 PM

Here is a link to that workflow run: https://github.com/ProfessorKolodziej/test-colors-ashleykolodziej/pull/1/checks?check_run_id=3562293250

Here is the code I have in GitHub actions that runs the action: https://github.com/ProfessorKolodziej/test-colors-ashleykolodziej/blob/main/.github/workflows/classroom.yml#L13

And here is the actual CLI command I'm using: https://github.com/ProfessorKolodziej/test-colors-ashleykolodziej/blob/main/package.json#L37

I was digging around to try and find out what this was all about, and I noticed that a similar issue came up in the main Chalk repo for false negatives in GitHub actions: https://github.com/chalk/supports-color/issues/106

It's since been closed, at least for GitHub actions. It looks like maybe this repo hasn't been updated in a while. Is it possible that updating the Chalk dependency on this repo could fix this?

Qix- commented 3 years ago

@sindresorhus

What is the intended behavior of this conditional? It's the culprit here. There is no great way to detect if the user wants stdin consumption without having them specify it.

https://github.com/chalk/chalk-cli/blob/932d798cdce67d9e28f4b40fa9446a5ae0a8a52c/cli.js#L78

The fix here would be to allow passing - as the "input" or otherwise passing a new --stdin, -s flag that indicates stdin should be used as the input. My preference is the latter as it's easier to discern between wanting stdin to parse and a literal - to display.

Qix- commented 3 years ago

Alright figured it out. To work around this, pass --no-stdin directly after chalk.

(You also need to add {stdio: 'inherit'} but that isn't due to this library in any way).

In your repro, that looks like this:

execSync('npx chalk --no-stdin yellow "expect my colorful log 1"', {stdio: 'inherit'});
> chalk-cli-issue-11@1.0.0 log
> node index.js

expect my colorful log 1

image


@sindresorhus I think meow is handling stdin all wrong, to be completely fair with you. --no-stdin is very much not a common unix pattern.

This is 1) undocumented as per our help docs, 2) not really a common unix-ey thing, 3) mishandles stdin if the option is accidentally omitted, clearly with a confusing effect. Not sure what the best course of action is aside from dropping meow and going with something a bit lighter weight.

I had also tagged this as "bug" but... I guess it's not, if we're going by how meow expects CLIs to work... not sure what to think about that.

ashleykolodziej commented 3 years ago

@Qix- the --no-stdin flag solved my problem in GitHub actions perfectly, thank you! I'll keep my eye on this issue for any future updates.

sindresorhus commented 3 years ago

I think meow is handling stdin all wrong

Meow doesn't deal with stdin at all. The handling of stdin is all in this package.

sindresorhus commented 3 years ago

@Qix- The main problem here is that both normal input and stdin input accept multiple arguments, so we cannot check the argument length, which is how I usually solve the ambiguity. The current behavior is actually the best one for human interaction with the CLI, but I can see how it's subpar when used for scripting. I think opt-in stdin would be best then --stdin. The - convention is just too magic.

msabramo commented 3 years ago

I've come back to this issue a number of times and there's not really a very short and sweet single-command way of reproducing the issue (I saw someone posted a git repo but I was too lazy to clone it 😄 ).

So here's a simple command that reproduces the issue:

echo '' | node cli.js red bold 'Some plain text to format'

With old versions of the code (e.g.: 932d798cdce67d9e28f4b40fa9446a5ae0a8a52c), this results in a strange Invalid style error:

$ git checkout -q 932d798cdce67d9e28f4b40fa9446a5ae0a8a52c && git log --oneline -n 1 && npm install --no-audit --no-fund
932d798 (HEAD, msabramo/main) Move to GitHub Actions (#15)
...

$ echo '' | node cli.js red bold 'Some plain text to format'
Invalid style: Some plain text to format

Since this was fixed in #17, it works fine on the main branch:

$ git checkout -q main && git log --oneline -n 1 && npm install --no-audit --no-fund
1dd4d97 (HEAD -> main, tag: v5.0.0) 5.0.0

up to date in 841ms

$ echo '' | node cli.js red bold 'Some plain text to format'
Some plain text to format