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.

Sign up for GitHub

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

Jump to bottom

Pretty the help text #10547

Merged
merged 1 commit into from Feb 4, 2015
Merged

Pretty the help text #10547

merged 1 commit into from Feb 4, 2015

Conversation

duglin
Copy link
Contributor

@duglin duglin commented Feb 4, 2015

This modifies the "docker help" text so that it is no wider than 80 chars
and each description fits on one line.

Added a test to make sure we don't go over 80 chars again.

https://www.youtube.com/watch?v=Kmv3WlKa6U8

Closes #10214

Signed-off-by: Doug Davis dug@us.ibm.com

@duglin duglin mentioned this pull request Feb 4, 2015
Closed
@SvenDowideit
Copy link
Contributor

LGTM

@phemmer
Copy link
Contributor

phemmer commented Feb 4, 2015

This looks much better, thanks :-)
Though this doesn't entirely close #10214. Other commands, such as docker run --help, or docker build --help have the same issue (though docker run --help is the biggest offender).

@duglin
Copy link
Contributor Author

duglin commented Feb 4, 2015

@phemmer oh, I read #10214 as just the main help text. Let me work on the others....

@duglin
Copy link
Contributor Author

duglin commented Feb 4, 2015

@phemmer ok done - everyone please give it a look.

Pretty the help text
2203b37 
This modifies the "docker help" text so that it is no wider than 80 chars
and each description fits on one line. This will also try to use ~ when
possible

Added a test to make sure we don't go over 80 chars again.
Added a test to make sure we use ~

Applied rules/tests to all docker commands - not just main help text

Closes moby#10214

Signed-off-by: Doug Davis <dug@us.ibm.com>
cpuguy83
cpuguy83 reviewed Feb 4, 2015
View reviewed changes
pkg/mflag/flag.go
@@ -503,6 +504,10 @@ func Set(name, value string) error {
// otherwise, the default values of all defined flags in the set.
func (f *FlagSet) PrintDefaults() {
writer := tabwriter.NewWriter(f.Out(), 20, 1, 3, ' ', 0)
var home string
if runtime.GOOS != "windows" {
Copy link
Member

Choose a reason for hiding this comment

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

See https://github.com/ahmetalpbalkan/docker/blob/homedir-fix/utils/homedir.go

Which is part of #10358, unmerged so far unfortunately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually don't want the HOME dir in general. On windows I want "home" to be "" because ~ wont' work on windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cpuguy83 that one is now #10602, sorry about that.

@crosbymichael
Copy link
Contributor

LGTM

1 similar comment
@jessfraz
Copy link
Contributor

jessfraz commented Feb 4, 2015

LGTM

jessfraz pushed a commit that referenced this pull request Feb 4, 2015
Merge pull request #10547 from duglin/Issue10214
a271eae 
Pretty the help text
@jessfraz jessfraz merged commit a271eae into moby:master Feb 4, 2015
@duglin duglin mentioned this pull request Feb 6, 2015
Merged
@SvenDowideit SvenDowideit mentioned this pull request Feb 9, 2015
Closed
@ahmetb
Copy link
Contributor

ahmetb commented Feb 18, 2015

Okay folks obviousy TestMainHelpWidth is failing on windows since it has to print full paths for cert paths etc. (e.g. --tlscacert="C:\\Users\\ahmetb\\.docker\\ca.pem")

  • Keep it ~ and leave what ~ means as an exercise to windows-people who never used unix before. But then you're printing something people can't execute by copy pasting.
  • Keep it ~ because ~ actually works in powershell. It's invalid in cmd.exe, probably what most people use. Maybe we can figure if we're running in powershell and print ~ but then we don't run CI tests in powershell, so that's probably not an option to get the test passing.
  • Skip this test, my favorite.

@duglin @jfrazelle

@duglin
Copy link
Contributor Author

duglin commented Feb 18, 2015

@ahmetalpbalkan would using %HOME% work/help?

@ahmetb
Copy link
Contributor

ahmetb commented Feb 18, 2015

@duglin not really. It's called %USERPROFILE%, though that's barely 1-2 chars shorter than my full c:\users\username path. I guess I'll try that. thanks.

@ahmetb
Copy link
Contributor

ahmetb commented Feb 18, 2015

it came as close as this (87 chars).

If I print default values without escaping (%q --> %s) this can save some unnecessary backslashes and quotes (but then rarely might cause problems in unix?) but it still has 3 chars extra:

@ahmetb ahmetb mentioned this pull request Feb 18, 2015
Merged
@SvenDowideit SvenDowideit mentioned this pull request Feb 26, 2015
Closed
@duglin duglin deleted the Issue10214 branch March 6, 2015 04:55
This was referenced Sep 25, 2016
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docker --help output too chatty
7 participants
@duglin @SvenDowideit @phemmer @crosbymichael @jessfraz @ahmetb @cpuguy83