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 readme usage instructions to emphasize Double Quotes for compatibility #509

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

victorhcb
Copy link

This discussion came up in a Brazilian project (TabNews) when a Pull Request revealed a compatibility issue with Windows due to the use of single quotes.

The idea of this PR is to make it clear in the documentation that using double quotes is the way to go if you want to avoid compatibility problems across different operating systems.

It’s a small change, but it could save users from wasting minutes—or even hours—trying to figure out what went wrong!

Copy link
Member

@gustavohenke gustavohenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey. I'm not sure if it's relevant to highlight it like that, as it's not concurrently specific, but rather how the shell parses inputs.

If we want to be thorough with the recommendations, then I'd argue that someone blindly changing their commands from single to double quotes could also face unintended side effects if they look like they have an env variable, e.g.

$ conc 'echo $foo'
[0]
[0] echo $foo exited with code 0
$ conc "echo $foo"
[0]
[0] echo  exited with code 0

May I suggest that maybe we change the note to say that the user should pick the right quotes for their use case?
Can highlight how Windows will only work with double ones, and that env vars in unix might be parsed eagerly depending on the style.

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

Successfully merging this pull request may close these issues.

2 participants