Add documentation about double quoting on Windows#4013
Conversation
On non-Unix-like shells like Windows Command Prompt, single quotes are handled differently. You need to define aliases using double quotes instead of single quotes. I added an inline example to illustrate the quotes. The example is formatted as inline code blocks in Markdown. Unfortunately, because Go uses backticks for raw string literals, I needed to do some rather ugly string concatenation in order to get the backticks included in the doc string. This also rearranges the notes so that the platform specific notes are at the end of the documentation.
The first paragraph uses single quotes when referring to shell arguments and variables, but the rest of the docs use double quotes. This commit switches to using single quotes throughout the docs. I prefer to use single quotes inside string literals because Go uses double quotes to define a string literal.
78f689b to
23ffca4
Compare
|
Hopefully, the new doc string is clear enough. Let me know if it isn't clear and I can redo it. Let me know your preferences on formatting as well. I really want to use an inline example that is formatted as inline code blocks in Markdown to make it clear how to use the double quotes since the main examples for the command all use single quotes. Unfortunately, because this is inside a raw string literal, I needed to do some ugly string concatenation to include the backticks in the doc string. Again, I think it would be really helpful to format it this way, but let me know if I should revise. |
mislav
left a comment
There was a problem hiding this comment.
Generally I think it's too much of a responsibility to teach users about their shell, but if this is a problem that people run into because they follow examples in our documentation, then I think it's a good idea that we add a note about it. Thanks for proposing this and thanks @vilmibm for helping convince me. 👍
pkg/cmd/alias/set/set.go
Outdated
|
|
||
| If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you | ||
| to compose commands with "|" or redirect with ">". Note that extra arguments following the alias | ||
| to compose commands with '|' or redirect with '>'. Note that extra arguments following the alias |
There was a problem hiding this comment.
I tried explaining it in the commit message for 23ffca4, but basically I changed it for consistency. The first paragraph uses single quotes to refer to things you'd type in the shell, but then later switches to double quotes to do the same thing.
Also, since this doc string is embedded in Go source code, I thought it'd be easier to use single quotes rather than double quotes.
pkg/cmd/alias/set/set.go
Outdated
|
|
||
| Platform note: on Windows, shell aliases are executed via "sh" as installed by Git For Windows. If | ||
| you have installed git on Windows in some other way, shell aliases may not work for you. | ||
| arguments, you must explicitly accept them using '$1', '$2', etc., or '$@' to accept all of them. |
There was a problem hiding this comment.
Why changing this to single quotes?
There was a problem hiding this comment.
Ditto from the previous comment. You'll notice the double quotes are used here, but in the first paragraph, single quotes are used to refer to the same variables.
pkg/cmd/alias/set/set.go
Outdated
| - Shell aliases are executed via 'sh' as installed by Git For Windows. | ||
| If you have installed git in some other way, shell aliases may not work for you. |
There was a problem hiding this comment.
I know that this sentence existed before, but we can maybe consider dropping it. Most GitHub users will have Git for Windows installed, and if they haven't, they will get an error message explaining that to gain sh.exe they should install Git for Windows.
There was a problem hiding this comment.
Sure, I'll remove it if you think the error message is clear enough.
pkg/cmd/alias/set/set.go
Outdated
|
|
||
| - Shell aliases are executed via 'sh' as installed by Git For Windows. | ||
| If you have installed git in some other way, shell aliases may not work for you. | ||
| - If you are using a shell that is *not* Unix-like (e.g. Command Prompt, cmd.exe), |
There was a problem hiding this comment.
I think we are literally just adding this note for cmd.exe users. I cannot think of a single other real-world use-case where someone would need to use double quotes instead of single quotes. So instead of saying something confusing like non-Unix-like shell, let's just say something like “Note for cmd.exe users: use double quotes instead of single quotes.” Keep it short and to the point.
There was a problem hiding this comment.
Ok, I can revise.
As discussed in issue cli#4013.
As discussed in pull request cli#4013.
c235881 to
1e4e536
Compare
mislav
left a comment
There was a problem hiding this comment.
Thank you! I've simplified the overall docs even further and moved the Command Prompt note to the Examples section next to the first usage of single quotes.
|
Great! Just curious, why do you prefer using double quotes to mark the shell arguments in the doc string? Also, I see you hard wrapped lines at about 100 characters. Is that what I should use if I revise docs in the future? |
|
@chemotaxis I prefer either double quotes or backticks for denoting values or user input in documentation. There is no strong reason, except that I'd rather that single “quotes” in prose are used mostly for apostrophes. Yes, we try to cap doc strings to ~80 columns. Together with leading indentation for these heredoc strings, that's about 100 columns. |
Fixes #3916.
To define an alias in a non-Unix-like shell (like Windows Command Prompt), you need to use double quotes. This also rearranges the notes so that the platform-specific notes are at the end of the documentation.