cli: additionalHelp() don't decorate output if it's piped, and add extra newline #3973
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This prevents the escape-characters being included when piping the
output, e.g. `docker --help > output.txt`, or `docker --help | something`.
These control-characters could cause issues if users copy/pasted the URL
from the output, resulting in them becoming part of the URL they tried
to visit, which would fail, e.g. when copying the output from:
To get more help with docker, check out our guides at https://docs.docker.com/go/guides/
Users ended up on URLs like;
https://docs.docker.com/go/guides/ESC
https://docs.docker.com/go/guides/%1B[0m
Before this patch, control characters ("bold") would be printed, even if
no TTY was attached;
docker --help > output.txt
cat output.txt | grep 'For more help' | od -c
0000000 033 [ 1 m F o r m o r e h e l
0000020 p o n h o w t o u s e
0000040 D o c k e r , h e a d t o
0000060 h t t p s : / / d o c s . d o c
0000100 k e r . c o m / g o / g u i d e
0000120 s / 033 [ 0 m \n
0000127
docker --help | grep 'For more help' | od -c
0000000 033 [ 1 m F o r m o r e h e l
0000020 p o n h o w t o u s e
0000040 D o c k e r , h e a d t o
0000060 h t t p s : / / d o c s . d o c
0000100 k e r . c o m / g o / g u i d e
0000120 s / 033 [ 0 m \n
0000127
With this patch, no control characters are included:
docker --help > output.txt
cat output.txt | grep 'For more help' | od -c
0000000 F o r m o r e h e l p o n
0000020 h o w t o u s e D o c k
0000040 e r , h e a d t o h t t p
0000060 s : / / d o c s . d o c k e r .
0000100 c o m / g o / g u i d e s / \n
0000117
docker --help | grep 'For more help' | od -c
0000000 F o r m o r e h e l p o n
0000020 h o w t o u s e D o c k
0000040 e r , h e a d t o h t t p
0000060 s : / / d o c s . d o c k e r .
0000100 c o m / g o / g u i d e s / \n
0000117
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The additionalHelp message is printed at the end of the --help output;
To get more help with docker, check out our guides at https://docs.docker.com/go/guides/
PS>
As this message may contain an URL, users may copy/paste the URL to open it
in their browser, but can easily end up copying their prompt (as there's
no whitespace after it), and as a result end up on a broken URL, for example:
https://docs.docker.com/go/guides/PS
This patch adds an extra newline at the end to provide some whitespace
around the message, making it less error-prone to copy the URL;
To get more help with docker, check out our guides at https://docs.docker.com/go/guides/
PS>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
commented
Jan 15, 2023
| if additionalHelp, ok := cmd.Annotations["additionalHelp"]; ok { | ||
| if msg, ok := cmd.Annotations["additionalHelp"]; ok { | ||
| out := cmd.OutOrStderr() | ||
| if _, isTerminal := term.GetFdInfo(out); !isTerminal { |
Member
Author
There was a problem hiding this comment.
Had this as a branch in my local repo for some time, but had to dust it off a bit (previously depended on #3971, but I rewrote it to detect the terminal slightly different)
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3973 +/- ##
===========================================
+ Coverage 0 59.19% +59.19%
===========================================
Files 0 287 +287
Lines 0 24693 +24693
===========================================
+ Hits 0 14617 +14617
- Misses 0 9192 +9192
- Partials 0 884 +884 |
vvoland
approved these changes
Jan 17, 2023
Member
Author
|
Thx! Let me bring this one in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to:
cli: additionalHelp() don't decorate output if it's piped
This prevents the escape-characters being included when piping the
output, e.g.
docker --help > output.txt, ordocker --help | something.These control-characters could cause issues if users copy/pasted the URL
from the output, resulting in them becoming part of the URL they tried
to visit, which would fail, e.g. when copying the output from:
Users ended up on URLs like;
Before this patch, control characters ("bold") would be printed, even if
no TTY was attached;
With this patch, no control characters are included:
Add extra newline after additionalHelp output
The additionalHelp message is printed at the end of the --help output;
As this message may contain an URL, users may copy/paste the URL to open it
in their browser, but can easily end up copying their prompt (as there's
no whitespace after it), and as a result end up on a broken URL, for example:
This patch adds an extra newline at the end to provide some whitespace
around the message, making it less error-prone to copy the URL;
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)