status-im / nim-confutils

Simplified handling of command line options and config files
Apache License 2.0
64 stars 16 forks source link

Fix alignment in help #71

Open vpavlin opened 1 year ago

vpavlin commented 1 year ago

Hi, first of all, I'd like to apologize for unnecessary formatting changes, let me know if you want me to try to avoid those to make the PR simpler.

This PR is an attempt to fix misalignments in help when there are options with and without abbr and various lengths of abbr and when there are long names of options.

I've initially noticed it with wakucanary CLI tool:

 wakucanary [OPTIONS]...

The following options are available:

 -a, --address                 Multiaddress of the peer node to attempt to dial.
 -t, --timeout                 Timeout to consider that the connection failed [=chronos.seconds(10)].
 -p, --protocol                Protocol required to be supported: store,relay,lightpush,filter (can be used
                               multiple times).
 -l, --log-level               Sets the log level [=LogLevel.DEBUG].
 -np, --node-port               Listening port for waku node [=60000].
     --websocket-secure-key-path  Secure websocket key path:   '/path/to/key.txt' .
     --websocket-secure-cert-path  Secure websocket Certificate path:   '/path/to/cert.txt' .

the output looks like this with the change:

The following options are available:

 -a,  --address                     Multiaddress of the peer node to attempt to dial.
 -t,  --timeout                     Timeout to consider that the connection failed.
 -p,  --protocol                    Protocol required to be supported: store,relay,lightpush,filter (can be used
                                   multiple times).
 -l,  --log-level                   Sets the log level.
 -np, --node-port                   Listening port for waku node [=60000].
      --websocket-secure-key-path   Secure websocket key path:   '/path/to/key.txt' .
      --websocket-secure-cert-path  Secure websocket Certificate path:   '/path/to/cert.txt' .

and this is an attempt to fix it and make everything aligned (I am sure it will need iterations).

arnetheduck commented 1 year ago
 -p,  --protocol                    Protocol required to be supported: store,relay,lightpush,filter (can be used
                                   multiple times).

off by one?

zah commented 1 year ago

I don't mind formatting fixes in pull requests, but I find the formatting choices being made here to be highly unusual.

Also, I can see that you are planning to use a two-letter abbreviation. I don't think this should be supported by the library, because in the future it intends to add support for the Unix convention of allowing multiple abbreviated options to be concatenated together (e.g. tar -xzfv). Multi-letter abbreviations can lead to ambiguities then.

Can you briefly describe what was the logic problem behind the fixed issue? Is it that the code was not taking the account the presence (or absence) of abbreviations when doing the padding width calculation?