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

wrap-desc: Wrap help descriptions under 80 chars #4121

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

devedge
Copy link
Contributor

@devedge devedge commented Aug 5, 2017

Make all help command descriptions wrap under 80 characters, as per issue #2783

License: MIT
Signed-off-by: Richard Pajerski devedge@outlook.com

Copy link
Contributor

@kevina kevina left a 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:

  1. 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

  2. some lines should be left long, espacally when they represent terminal output

@whyrusleeping, others thoughts

@@ -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
:)"]'

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@@ -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
Copy link
Contributor

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.

@whyrusleeping
Copy link
Member

Yeah, agreed on using spaces instead of tabs, ensures that the output looks the same for everyone.

Copy link
Contributor

@kevina kevina left a 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

@@ -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'):

Copy link
Contributor

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")

@whyrusleeping
Copy link
Member

I think two spaces is fine. @devedge if you can get the last comment from @kevina addressed then i think this will be ready to merge

@devedge
Copy link
Contributor Author

devedge commented Sep 13, 2017

Alright, I'll have it up by tonight

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>
Copy link
Contributor

@kevina kevina left a comment

Choose a reason for hiding this comment

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

This LGTM now.

@whyrusleeping
Copy link
Member

will merge post 0.4.11 release :)

Thanks @devedge

@whyrusleeping whyrusleeping added this to the Ipfs 0.4.12 milestone Sep 21, 2017
@whyrusleeping whyrusleeping merged commit d653429 into ipfs:master Oct 31, 2017
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.

None yet

3 participants