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

improve help text on narrow terminals #140

Merged
merged 8 commits into from
May 14, 2019
Merged

improve help text on narrow terminals #140

merged 8 commits into from
May 14, 2019

Conversation

Stebalien
Copy link
Member

  1. Wrap flag and argument descriptions.
  2. Wrap the synopsys (should we add a backslash to the end of each line?)
  3. Don't align flags in columns to save space.

ipfs add now looks like:

USAGE
  ipfs add <path>... - Add a file or directory to ipfs.

SYNOPSIS
  ipfs add [--recursive | -r] [--dereference-args] [--quiet | -q]
           [--quieter | -Q] [--silent] [--progress | -p] [--trickle | -t]
           [--only-hash | -n] [--wrap-with-directory | -w]
           [--stdin-name=<stdin-name>] [--hidden | -H]
           [--chunker=<chunker> | -s] [--pin=false] [--raw-leaves] [--nocopy]
           [--fscache] [--cid-version=<cid-version>] [--hash=<hash>] [--inline]
           [--inline-limit=<inline-limit>] [--] <path>...

ARGUMENTS

  <path>... - The path to a file to be added to ipfs.

OPTIONS

  -r, --recursive           bool   - Add directory paths recursively.
  --dereference-args        bool   - Symlinks supplied in arguments are
                                     dereferenced.
  -q, --quiet               bool   - Write minimal output.
  -Q, --quieter             bool   - Write only final hash.
  --silent                  bool   - Write no output.
  -p, --progress            bool   - Stream progress data.
  -t, --trickle             bool   - Use trickle-dag format for dag generation.
  -n, --only-hash           bool   - Only chunk and hash - do not write to disk.
  -w, --wrap-with-directory bool   - Wrap files with a directory object.
  --stdin-name              string - Assign a name if the file source is stdin.
  -H, --hidden              bool   - Include files that are hidden. Only takes
                                     effect on recursive add.
  -s, --chunker             string - Chunking algorithm, size-[bytes] or
                                     rabin-[min]-[avg]-[max]. Default:
                                     size-262144.
  --pin                     bool   - Pin this object when adding. Default: true.
  --raw-leaves              bool   - Use raw blocks for leaf nodes.
                                     (experimental).
  --nocopy                  bool   - Add the file using filestore. Implies
                                     raw-leaves. (experimental).
  --fscache                 bool   - Check the filestore for pre-existing
                                     blocks. (experimental).
  --cid-version             int    - CID version. Defaults to 0 unless an
                                     option that depends on CIDv1 is passed.
                                     (experimental).
  --hash                    string - Hash function to use. Implies CIDv1 if not
                                     sha2-256. (experimental). Default:
                                     sha2-256.
  --inline                  bool   - Inline small blocks into CIDs.
                                     (experimental).
  --inline-limit            int    - Maximum block size to inline.
                                     (experimental). Default: 32.

Was:

USAGE
  ipfs add <path>... - Add a file or directory to ipfs.

SYNOPSIS
  ipfs add [--recursive | -r] [--dereference-args] [--quiet | -q] [--quieter | -Q] [--silent] [--progress | -p] [--trickle | -t] [--only-hash | -n] [--wrap-with-directory | -w] [--stdin-name=<stdin-name>] [--hidden | -H] [--chunker=<chunker> | -s] [--pin=false] [--raw-leaves] [--nocopy] [--fscache] [--cid-version=<cid-version>] [--hash=<hash>] [--inline] [--inline-limit=<inline-limit>] [--] <path>...

ARGUMENTS

  <path>... - The path to a file to be added to ipfs.

OPTIONS

  -r,               --recursive           bool   - Add directory paths recursively.
  --dereference-args                      bool   - Symlinks supplied in arguments are dereferenced.
  -q,               --quiet               bool   - Write minimal output.
  -Q,               --quieter             bool   - Write only final hash.
  --silent                                bool   - Write no output.
  -p,               --progress            bool   - Stream progress data.
  -t,               --trickle             bool   - Use trickle-dag format for dag generation.
  -n,               --only-hash           bool   - Only chunk and hash - do not write to disk.
  -w,               --wrap-with-directory bool   - Wrap files with a directory object.
  --stdin-name                            string - Assign a name if the file source is stdin.
  -H,               --hidden              bool   - Include files that are hidden. Only takes effect on recursive add.
  -s,               --chunker             string - Chunking algorithm, size-[bytes] or rabin-[min]-[avg]-[max]. Default: size-262144.
  --pin                                   bool   - Pin this object when adding. Default: true.
  --raw-leaves                            bool   - Use raw blocks for leaf nodes. (experimental).
  --nocopy                                bool   - Add the file using filestore. Implies raw-leaves. (experimental).
  --fscache                               bool   - Check the filestore for pre-existing blocks. (experimental).
  --cid-version                           int    - CID version. Defaults to 0 unless an option that depends on CIDv1 is passed. (experimental).
  --hash                                  string - Hash function to use. Implies CIDv1 if not sha2-256. (experimental). Default: sha2-256.
  --inline                                bool   - Inline small blocks into CIDs. (experimental).
  --inline-limit                          int    - Maximum block size to inline. (experimental). Default: 32.

@Stebalien Stebalien requested a review from keks as a code owner January 11, 2019 05:26
@ghost ghost assigned Stebalien Jan 11, 2019
@ghost ghost added the status/in-progress In progress label Jan 11, 2019
Copy link
Member

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

This is much better!

@magik6k
Copy link
Member

magik6k commented Jan 11, 2019

should we add a backslash to the end of each line?

Lack of - before wrapped lines should be enough

@kevina
Copy link
Contributor

kevina commented Jan 11, 2019

I'll look over this more closely later today but overall I like it.

It would be good to get the width of the terminal, but there doesn't seam to be a standard way to do this so this is something that could come later.

@Stebalien
Copy link
Member Author

It would be good to get the width of the terminal, but there doesn't seam to be a standard way to do this so this is something that could come later.

When we do this, we also need to reflow the command description to match the terminal width (maxing out at some reasonable width).

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 like the idea but this needs some polish.

  • The SYNOPSIS for the root command is not wrapped:
USAGE
  ipfs - Global p2p merkle-dag filesystem.

SYNOPSIS
  ipfs [--config=<config> | -c] [--debug=<debug> | -D] [--help=<help>] [-h=<h>] [--local=<local> | -L] [--api=<api>] <command> ...
  • When displaying sub commands, the description should wrap also, for example ipfs filestore --help:
SUBCOMMANDS
  ipfs filestore dups              - List blocks that are both in the filestore and standard block storage.
  ipfs filestore ls [<obj>]...     - List objects in filestore.
  ipfs filestore verify [<obj>]... - Verify objects in filestore.

  Use 'ipfs filestore <subcmd> --help' for more information about each command.
  • In general we should dump the output of the help text for each sub command and look over it so it looks reasonable.

@Stebalien
Copy link
Member Author

The SYNOPSIS for the root command is not wrapped:

Damn. It's custom text. Options:

  1. Parse it and reflow.
  2. Add newlines to the root command in go-ipfs itself.

When displaying sub commands, the description should wrap also, for example

👍

@kevina
Copy link
Contributor

kevina commented Jan 14, 2019

It would be good to get the width of the terminal, but there doesn't seam to be a standard way to do this so this is something that could come later.

When we do this, we also need to reflow the command description to match the terminal width (maxing out at some reasonable width).

Yeah, I didn't think about the fact that description text is already formatted for 80 cols. Hard coding the output to 80 cols makes sense for now.

  1. Add newlines to the root command in go-ipfs itself.

I think that is the easiest option.

@Stebalien
Copy link
Member Author

Wrapped the subcommand text. We can fix the the root's synopsis when we merge this into go-ipfs.

USAGE
  ipfs filestore - Interact with filestore objects.

SYNOPSIS
  ipfs filestore

SUBCOMMANDS
  ipfs filestore dups              - List blocks that are both in the filestore
                                     and standard block storage.
  ipfs filestore ls [<obj>]...     - List objects in filestore.
  ipfs filestore verify [<obj>]... - Verify objects in filestore.

  Use 'ipfs filestore <subcmd> --help' for more information about each command.

@kevina
Copy link
Contributor

kevina commented Jan 15, 2019

USAGE section should also likely be wrapped:

USAGE
  ipfs filestore dups - List blocks that are both in the filestore and standard block storage.
                                                                                xxxxxx

So should the SUBCOMMANDS footer:

  Use 'ipfs bootstrap add <subcmd> --help' for more information about each command.
                                                                                xxxxxx
  Use 'ipfs config profile <subcmd> --help' for more information about each command.
                                                                                xxxxxx

ipfs daemon options are not wrapping:

  --init                         bool   - Initialize ipfs with default settings if not already initialized.
  --init-profile                 string - Configuration profiles to apply for --init. See ipfs init --help for more.
  --routing                      string - Overrides the routing option. Default: default.
  --mount                        bool   - Mounts IPFS to the filesystem.
  --writable                     bool   - Enable writing objects (with POST, PUT and DELETE).
  --mount-ipfs                   string - Path to the mountpoint for IPFS (if using --mount). Defaults to config setting
  --mount-ipns                   string - Path to the mountpoint for IPNS (if using --mount). Defaults to config setting
  --unrestricted-api             bool   - Allow API access to unlisted hashes.
  --disable-transport-encryption bool   - Disable transport encryption (for debugging protocols).
  --enable-gc                    bool   - Enable automatic periodic repo garbage collection.
  --manage-fdlimit               bool   - Check and raise file descriptor limits if needed. Default: true.
  --migrate                      bool   - If true, assume yes at the migrate prompt. If false, assume no.
  --enable-pubsub-experiment     bool   - Instantiate the ipfs daemon with the experimental pubsub feature enabled.
  --enable-namesys-pubsub        bool   - Enable IPNS record distribution through pubsub; enables pubsub.
  --enable-mplex-experiment      bool   - Add the experimental 'go-multiplex' stream muxer to libp2p on construction. De

ipfs p2p subcommand need help:

SUBCOMMANDS
  ipfs p2p close                                                - Stop listening for new connections to forward.
  ipfs p2p forward <protocol> <listen-address> <target-address> - Forward connections to libp2p service
  ipfs p2p listen <protocol> <target-address>                   - Create libp2p service
  ipfs p2p ls                                                   - List active p2p listeners.
  ipfs p2p stream                                               - P2P stream management.

And some additional manual cleanup is required on ipfs config show:

DESCRIPTION

  NOTE: For security reasons, this command will omit your private key. If you would like to make a full backup of your config (private key included), you must copy the config 

Easy way to dump all help text:

ipfs --help && { ipfs commands | while read l; do $l --help ; done } | less -S`

(but note it is slightly dangerous as it is directly executing each line of the ipfs commands output)

@kevina
Copy link
Contributor

kevina commented Jan 15, 2019

One other minor nit, how the type field is presented is slightly confusing and causes me to have to pause a second to figure it out that it not another option:

  -r, --recursive           bool   - Add directory paths recursively.
  --dereference-args        bool   - Symlinks supplied in arguments are
                                     dereferenced.
  -q, --quiet               bool   - Write minimal output.
  -Q, --quieter             bool   - Write only final hash.
  --silent                  bool   - Write no output.
  -p, --progress            bool   - Stream progress data.
  -t, --trickle             bool   - Use trickle-dag format for dag generation.
  -n, --only-hash           bool   - Only chunk and hash - do not write to disk.
  -w, --wrap-with-directory bool   - Wrap files with a directory object.
  --stdin-name              string - Assign a name if the file source is stdin.
  -H, --hidden              bool   - Include files that are hidden. Only takes
                                     effect on recursive add.
  -s, --chunker             string - Chunking algorithm, size-[bytes] or
                                     rabin-[min]-[avg]-[max]. Default:
                                     size-262144.

Two suggestions (1) remove the space between the , and the next option. or (2) add an extra space between the option and the type field. What (1) might look like:

  -r,--recursive           bool   - Add directory paths recursively.
  --dereference-args       bool   - Symlinks supplied in arguments are
                                    dereferenced.
  -q,--quiet               bool   - Write minimal output.
  -Q,--quieter             bool   - Write only final hash.
  --silent                 bool   - Write no output.
  -p,--progress            bool   - Stream progress data.
  -t,--trickle             bool   - Use trickle-dag format for dag generation.
  -n,--only-hash           bool   - Only chunk and hash - do not write to disk.
  -w,--wrap-with-directory bool   - Wrap files with a directory object.
  --stdin-name             string - Assign a name if the file source is stdin.
  -H,--hidden              bool   - Include files that are hidden. Only takes
                                    effect on recursive add.
  -s,--chunker             string - Chunking algorithm, size-[bytes] or
                                    rabin-[min]-[avg]-[max]. Default:
                                    size-262144.

What (2) might look like:

  -r, --recursive            bool   - Add directory paths recursively.
  --dereference-args         bool   - Symlinks supplied in arguments are
                                      dereferenced.
  -q, --quiet                bool   - Write minimal output.
  -Q, --quieter              bool   - Write only final hash.
  --silent                   bool   - Write no output.
  -p, --progress             bool   - Stream progress data.
  -t, --trickle              bool   - Use trickle-dag format for dag generation.
  -n, --only-hash            bool   - Only chunk and hash - do not write to
                                      disk.
  -w, --wrap-with-directory  bool   - Wrap files with a directory object.
  --stdin-name               string - Assign a name if the file source is stdin.
  -H, --hidden               bool   - Include files that are hidden. Only takes
                                      effect on recursive add.
  -s, --chunker              string - Chunking algorithm, size-[bytes] or
                                      rabin-[min]-[avg]-[max]. Default:
                                      size-262144.

(1) saves space but I think (2) looks better, another option to save even more space (option (3))

  -r,--recursive           bool   Add directory paths recursively.
  --dereference-args       bool   Symlinks supplied in arguments are
                                  dereferenced.
  -q,--quiet               bool   Write minimal output.
  -Q,--quieter             bool   Write only final hash.
  --silent                 bool   Write no output.
  -p,--progress            bool   Stream progress data.
  -t,--trickle             bool   Use trickle-dag format for dag generation.
  -n,--only-hash           bool   Only chunk and hash - do not write to disk.
  -w,--wrap-with-directory bool   Wrap files with a directory object.
  --stdin-name             string Assign a name if the file source is stdin.
  -H,--hidden              bool   Include files that are hidden. Only takes
                                  effect on recursive add.
  -s,--chunker             string Chunking algorithm, size-[bytes] or
                                  rabin-[min]-[avg]-[max]. Default:
                                  size-262144.

I actually like (3) the best. Thoughts?

@Stebalien Stebalien force-pushed the feat/wrap-text branch 2 times, most recently from faa3c14 to f4e8230 Compare January 17, 2019 15:49
@Stebalien
Copy link
Member Author

ipfs p2p subcommand need help:
ipfs daemon options are not wrapping:

I've lowered the minimum description width to 30 characters and raised the terminal width to 100 characters (nobody uses 80 character terminals anymore anyways).

This was happening because we stop wrapping if we have less than 30 characters to work with for the description (we don't want one word on each line).

@Stebalien
Copy link
Member Author

(1) saves space but I think (2) looks better, another option to save even more space (option (3))

I've gone with option 1. We can consider removing - everywhere later.

So should the SUBCOMMANDS footer:

I've rewritten the footer text to avoid this.

USAGE section should also likely be wrapped:

Done.


> ipfs p2p --help
USAGE
  ipfs p2p - Libp2p stream mounting.

SYNOPSIS
  ipfs p2p

DESCRIPTION

  Create and use tunnels to remote peers over libp2p
  
  Note: this command is experimental and subject to change as usecases and APIs
  are refined

SUBCOMMANDS
  ipfs p2p close                                                - Stop listening for new
                                                                  connections to forward.
  ipfs p2p forward <protocol> <listen-address> <target-address> - Forward connections to libp2p
                                                                  service
  ipfs p2p listen <protocol> <target-address>                   - Create libp2p service
  ipfs p2p ls                                                   - List active p2p listeners.
  ipfs p2p stream                                               - P2P stream management.

  For more information about each command, use:
  'ipfs p2p <subcmd> --help'

@kevina
Copy link
Contributor

kevina commented Jan 17, 2019

and raised the terminal width to 100 characters (nobody uses 80 character terminals anymore anyways).

While it is true that nobody really uses 80, 80 is a universal (and safe) default. Many terminals still default to 80 charters wide until resizes. Hard coding anything other than 80 makes me unconformable.

May I suggest something like:

  ipfs p2p close              - Stop listening for new connections to forward.
  ipfs p2p forward <protocol> <listen-address> <target-address>
                              - Forward connections to libp2p service
  ipfs p2p listen <protocol> <target-address>
                              - Create libp2p service
  ipfs p2p ls                 - List active p2p listeners.
  ipfs p2p stream             - P2P stream management.

That is if the command description does not start at say column 30, push it down to the next line. That is how output for other command or often formatted. For example ipfs ls --help:

...
  -a, --all                  do not ignore entries starting with .
  -A, --almost-all           do not list implied . and ..
      --author               with -l, print the author of each file
  -b, --escape               print C-style escapes for nongraphic characters
      --block-size=SIZE      scale sizes by SIZE before printing them; e.g.,
                               '--block-size=M' prints sizes in units of
                               1,048,576 bytes; see SIZE format below
  -B, --ignore-backups       do not list implied entries ending with ~
  -c                         with -lt: sort by, and show, ctime (time of last
                               modification of file status information);
                               with -l: show ctime and sort by name;
                               otherwise: sort by ctime, newest first
  -C                         list entries by columns
      --color[=WHEN]         colorize the output; WHEN can be 'never', 'auto',
                               or 'always' (the default); more info below
  -d, --directory            list directories themselves, not their contents
  -D, --dired                generate output designed for Emacs' dired mode
  -f                         do not sort, enable -aU, disable -ls --color
  -F, --classify             append indicator (one of */=>@|) to entries
      --file-type            likewise, except do not append '*'
      --format=WORD          across -x, commas -m, horizontal -x, long -l,
                               single-column -1, verbose -l, vertical -C
      --full-time            like -l --time-style=full-iso
  -g                         like -l, but do not list owner
      --group-directories-first
                             group directories before files;
                               can be augmented with a --sort option, but any
                               use of --sort=none (-U) disables grouping
  -G, --no-group             in a long listing, don't print group names
  -h, --human-readable       with -l and/or -s, print human readable sizes
                               (e.g., 1K 234M 2G)
      --si                   likewise, but use powers of 1000 not 1024
  -H, --dereference-command-line
                             follow symbolic links listed on the command line
      --dereference-command-line-symlink-to-dir
                             follow each command line symbolic link
                               that points to a directory
      --hide=PATTERN         do not list implied entries matching shell PATTERN
                               (overridden by -a or -A)
      --indicator-style=WORD  append indicator with style WORD to entry names:
                               none (default), slash (-p),
                               file-type (--file-type), classify (-F)
  -i, --inode                print the index number of each file
...

@Stebalien
Copy link
Member Author

Mind leaving that for a future PR? IMO, this PR is strictly better than what we currently have and it's not really a priority (I can't dedicate any more time to it).

@kevina
Copy link
Contributor

kevina commented Jan 17, 2019

Mind leaving that for a future PR? IMO, this PR is strictly better than what we currently have and it's not really a priority (I can't dedicate any more time to it).

I would say if we set the default width back to 80 then yes.

I can also take over this P.R, but it won't be to next week.

Instead of aligning flags in columns, display them right next to each other.
Otherwise, they take up *way* too much space.
We could be fancier and figure out the terminal width but wrapping at a fixed
80c is a decent start.
Otherwise, `ipfs daemon --help` and `ipfs p2p --help` look terrible.
@hannahhoward hannahhoward force-pushed the feat/wrap-text branch 2 times, most recently from eae0186 to 000220a Compare May 14, 2019 20:18
@hannahhoward
Copy link
Contributor

hannahhoward commented May 14, 2019

@Stebalien I was gonna go ahead and fix ipfs/kubo#340 -- and I found this along the way and I think improved it at least to the point where it should resolve previous objects.

The default width is now 80, and terminal width is detected using a semi-standard-ish package (it's from golang.org, even if it's a supplemental part of the crypto package)

If the io.writer given to help text is the terminal, read its width. otherwise, use a default.
@Stebalien
Copy link
Member Author

✔️ (can't approve my own PR)

@Stebalien Stebalien merged commit 1995f5c into master May 14, 2019
@Stebalien Stebalien deleted the feat/wrap-text branch May 14, 2019 23:19
@Stebalien Stebalien removed the status/in-progress In progress label May 14, 2019
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.

4 participants