-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
wrap-desc: Wrap help descriptions under 80 chars #4121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few notes, I do not speak for others, but I think:
-
tabs should be replaces with 2, 3, or 4 spaces, a width of a tab is not fixed so it is difficult to wrap correctly
-
some lines should be left long, espacally when they represent terminal output
@whyrusleeping, others thoughts
cmd/ipfs/daemon.go
Outdated
@@ -92,7 +92,8 @@ do this by setting headers on the API.HTTPHeaders and Gateway.HTTPHeaders | |||
keys: | |||
|
|||
ipfs config --json API.HTTPHeaders.X-Special-Header '["so special :)"]' | |||
ipfs config --json Gateway.HTTPHeaders.X-Special-Header '["so special :)"]' | |||
ipfs config --json Gateway.HTTPHeaders.X-Special-Header '["so special | |||
:)"]' | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably not a good idea to wrap here.
I personally would remove the tab and replace it with two (maybe up to four) spaces to help solve the problem to keep it all on a single line.
As an aside I would not use tabs in help text as the width of tab characters is not fixed.
But I may not speak for others. @whyrusleeping thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, there were a few places like this throughout the docs I can revert back.
core/commands/swarm.go
Outdated
@@ -369,7 +370,8 @@ var swarmDisconnectCmd = &cmds.Command{ | |||
'ipfs swarm disconnect' closes a connection to a peer address. The address | |||
format is an IPFS multiaddr: | |||
|
|||
ipfs swarm disconnect /ip4/104.131.131.82/tcp/4001/ipfs/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ | |||
ipfs swarm disconnect /ip4/104.131.131.82/tcp/4001/ipfs/QmaCpDMGvV2BGHeYERUEnR | |||
QAwe3N8SzbUtfsmvsqQLuvuJ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not split a hash like this. put it on its on line, if it still too long then I would leave it.
51972ec
to
73b7103
Compare
Yeah, agreed on using spaces instead of tabs, ensures that the output looks the same for everyone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I think about it using 2 spaces is probably not obvious enough. @whyrusleeping what do you think? I would go with 3 spaces, but 4 spaces would be ok.
Example (2 spaces):
experimental alternative that operates the DHT in a 'client only' mode that
can be enabled by running the daemon as:
ipfs daemon --routing=dhtclient
Three spaces:
experimental alternative that operates the DHT in a 'client only' mode that
can be enabled by running the daemon as:
ipfs daemon --routing=dhtclient
Four spaces:
experimental alternative that operates the DHT in a 'client only' mode that
can be enabled by running the daemon as:
ipfs daemon --routing=dhtclient
core/commands/publish.go
Outdated
@@ -49,7 +50,8 @@ Publish an <ipfs-path> with another name, added by an 'ipfs key' command: | |||
> ipfs name publish --key=mykey /ipfs/QmatmE9msSfkKxoffpHwNLNKgwZG8eT9Bud6YoPab52vpy | |||
Published to QmbCMUZw6JFeZ7Wp9jkzbye3Fzp2GGcPgC3nmeUjfVF87n: /ipfs/QmatmE9msSfkKxoffpHwNLNKgwZG8eT9Bud6YoPab52vpy | |||
|
|||
Alternatively, publish an <ipfs-path> using a valid PeerID(as listed by 'ipfs key list -l'): | |||
Alternatively, publish an <ipfs-path> using a valid PeerID(as listed by 'ipfs | |||
key list -l'): | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this two:
Alternatively, publish an <ipfs-path> using a valid PeerID (as listed by
'ipfs key list -l'):
To avoid splitting the command string (and also add a space after "PeerID")
Alright, I'll have it up by tonight |
73b7103
to
1575849
Compare
Wraps the description sections under 80 characters. Updated commit to adhere to requested changes. License: MIT Signed-off-by: Richard Pajerski II <devedge@outlook.com>
1575849
to
07d2b64
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM now.
will merge post 0.4.11 release :) Thanks @devedge |
Make all help command descriptions wrap under 80 characters, as per issue #2783
License: MIT
Signed-off-by: Richard Pajerski devedge@outlook.com