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 parameter's default #190

Closed
vitreo12 opened this issue Feb 27, 2021 · 5 comments
Closed

Wrap parameter's default #190

vitreo12 opened this issue Feb 27, 2021 · 5 comments

Comments

@vitreo12
Copy link

Hello,

Sorry to bother again. I can't find a way to wrap around a very long "default" value for a parameter:

Usage:
  omnicollider [optional-params] [files: string...]
Options:
  -h, --help                                                                                              print this
                                                                                                          cligen-erated
                                                                                                          help
  --help-syntax                                                                                           advanced:
                                                                                                          prepend,plurals,..
  -v, --version           bool    false                                                                   print version
  -o=, --outDir=          string  "~/.local/share/SuperCollider/Extensions"                               Output
                                                                                                          directory.
                                                                                                          Defaults to
                                                                                                          SuperCollider's
                                                                                                          "Platform.userExtensionDir".
  -p=, --scPath=          string  "~/.nimble/pkgs/omnicollider-0.3.0/omnicolliderpkg/deps/supercollider"  Path to the
                                                                                                          SuperCollider
                                                                                                          source code
                                                                                                          folder. Defaults
                                                                                                          to the one in
                                                                                                          omnicollider's
                                                                                                          dependencies.
  -a=, --architecture=    string  "native"                                                                Build
                                                                                                          architecture.
  -s, --supernova         bool    true                                                                    Build with
                                                                                                          supernova
                                                                                                          support.
  -r, --removeBuildFiles  bool    true                                                                    Remove all
                                                                                                          source files
                                                                                                          used for
                                                                                                          compilation from
                                                                                                          outDir.

Here I'd like to wrap the "~/.nimble/pkgs/omnicollider-0.3.0/omnicolliderpkg/deps/supercollider" string in order for the printed help file to be more readable.

Is this possible somehow?

@c-blake
Copy link
Owner

c-blake commented Feb 27, 2021

I agree there are occasionally some long default values (mostly for string, but also for collection types like seq[T], HashSet[T], etc.) that could benefit from a wrapping system for default values.

Presently, there is a way to drop the whole column (test/HelpTabCols.nim shows how) and the final column is auto-wrappable (on whitespace breaks) which might not help in your specific situation, but is adjacent. You can, of course, just hard-code the entire help output by setting usage. This abandons all automation and its benefits. Let's think about what we would need to add this auto-wrap-the-default-value-column feature, though.

Wrapping requires a max printed width, currently specifiable only for the final column in cligen/textUt.alignTable. This would be expressed best as a percentage or fraction (like saying either 16|20%) of the full width (80 columns). It would not be hard to add the ability to set this for the default value column and just "wrap like a terminal" with no smart breaking.

Wrapping with smart breaking requires a way to split the string into words and re-join them. Right now, for the description column only, cligen/textUt.wrap just does a naive split on a hard coded set[char] of white space. In your case, it would probably be (at least) strutils.Whitespace + {'/'}, but it could be more general (e.g. maybe add other punctuation). It would not be hard to generalize this set, but "stitching back together" whitespace separated stuff is easy - you just add a single space char and this conveniently folds all the various whitespace chars into one. If the delimiter is just for breaking, like / might be in your case, you need to preserve the character..trickier. So, just expanding the set[char] is not enough. Probably you need two set[char] - squashed space and preserved delimiters.

In natural language, smart breaking can get even more complex. Beyond that, a cligen-specific problem is that "soft-hyphens" (like writing "mech-anism" and having the system auto-remove the hyphen when unneeded) aren't really possible for the default value column. That value is controlled by the API author not the CLauthor. (It would help for the description column controlled by the CLauthor.) So, doing soft-hyphenation for default values would mean the CLauthor would have to copy-paste the default value and soft-hyphenate it -- defeating the "automation keeps few points of edit" value-adds of the package. So, auto-hyphenation (at least of default values) is probably just out of scope for cligen. Probably out of scope for description column, too, but maybe never say never.

Finally, there are questions about how either CLauthors and/or CLusers via the $CLIGEN config file should specify things (only users know how narrow/wide their terminals are usually in practice). For just a max width with "dumb terminal wrap" that is not so bad - it should clearly just be a global setting for the whole table. Breaking punctuation, however, needs to be per-parameter since some overlong ones might be filesystem paths wanting / while others like seq[T] could want ,.

What do you think? It would make sense (& add some initial value) to just do dumb terminal wrapping to be later superceded by smarter punctuation wrapping with a jagged-edge-minimizer. I am a bit busy with other things to do this right away (or even likely before the next cligen release in a few days), but I am open to pull requests if you (or any onlooker!) want to try.

@vitreo12
Copy link
Author

Thanks for the thorough answer.

I'd say that it would make sense to just do the "dumb terminal wrapping", as this feature might be a niche for specific use cases; a quick and dirty solution should fit the bill. I'd put up a PR myself but I have some deadlines at the moment, I'll keep the issue in mind to contribute in the future!

@c-blake
Copy link
Owner

c-blake commented Feb 27, 2021

Ok. Noted. FWIW, I was just updating textUt.alignTable the other week and, IIRC, it will need a bit of an overhaul to do even dumb terminal wrapping since I believe there was a strong, used/simplifying assumption of the only wrapped column being the final one.

@vitreo12
Copy link
Author

I see. Perhaps this is something for a future release. I came up with a "dirty" solution by simply having the "paths" embedded in the "help" string, instead that in the default:

OmniCollider - version 0.3.0
(c) 2020-2021 Francesco Cameli

Arguments:
  Omni file(s) or folder.

Options:
  -h, --help                                print this cligen-erated help
  -v, --version           bool    false     print version
  -o=, --outDir=          string  ""        Output directory. Defaults to SuperCollider's "Platform.userExtensionDir":
                                            "~/.local/share/SuperCollider/Extensions".
  -p=, --scPath=          string  ""        Path to the SuperCollider source code folder.
                                            Defaults to the one in OmniCollider's dependencies:
                                            "~/.nimble/pkgs/omnicollider-0.3.0/omnicolliderpkg/deps/supercollider".
  -a=, --architecture=    string  "native"  Build architecture.
  -s, --supernova         bool    true      Build with supernova support.
  -r, --removeBuildFiles  bool    true      Remove all source files used for compilation from outDir.

@c-blake
Copy link
Owner

c-blake commented Feb 28, 2021

Yeah. I have done that exact same trick myself at least once or twice. Very easy to check "" or .len==0 in the code and default there. I should've mentioned that as a possible workaround.

The complexity in textUt.alignTable is mostly that currently since only the final aka right-most column can wrap, you do not have to "measure the number of rows" of any prior column to see how tall a row is. So, in this case the move from "just 1,the last" to "earlier and/or the last" re-organizes the entire computation/almost the whole proc really. That's why I didn't just add some ClCfg max cols formatting stuff and tweak it in..needs almost a whole re-write and re-testing. None of it is rocket science, but it's probably a bit of work/head scratching. Not even counting trying to leave room for fancy punctutation-based work breaks

@c-blake c-blake closed this as completed in 8dc6449 Mar 4, 2021
c-blake added a commit that referenced this issue May 27, 2021
exclusivity of selection.

Solving the whole issue touches #190
which seems to need a re-write of `textUt.alignTable` to be a much smarter
layout engine (likely also involving more docs about user-controls).
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

No branches or pull requests

2 participants