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

FR: Display textual representations of available enum values in the clValType column #195

Closed
ZoomRmc opened this issue May 24, 2021 · 9 comments

Comments

@ZoomRmc
Copy link
Contributor

ZoomRmc commented May 24, 2021

Currently clValType displays the type name of the enum arguments, which is of questionable value for the user.
I propose displaying textual representations of the available enumerators in this column.
Also, it would be nice to replace generic "Set X" with "Select X" or "Choose X" for enums.

Mock-up:

type Dir enum = East = "E", West = "W", North ="N", South = "S"

-d=, --direction= E|W|N|S E Select direction

@c-blake
Copy link
Owner

c-blake commented May 25, 2021

So, the set -> select is fine, but the problem with listing all enums in a help table column is that there is not much room. You can usually pass an empty string for such an option to get a full list as an error message. Tricks like this can perhaps be in --help-syntax output.

@c-blake
Copy link
Owner

c-blake commented May 26, 2021

I should have said that also the default value column does not wrap. What I usually do is manually enter the list of enum values in the final, wrappable description column. (This could perhaps be semi-automated by providing an interpolation variable for description text, expanding to a ,-joined list of enum values when used.)

@c-blake
Copy link
Owner

c-blake commented May 27, 2021

BTW, I am open to that interpolation variable for the value of help[key for an enum type] and/or an line in the default help-syntax if you want to work on a PR. Just some select 1 of $1 kind of thing with classic %. That would be the easiest semi-automatic path forward, IMO. (You can, of course, override the entire text for --help-syntax in your cligen config file. So, that is lower priority.)

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented May 27, 2021

I think you're right to close this issue, however, one thing to consider is if you're really fine with exposing internal names to the user by default.

Regarding your latest comment, I'm not familiar with cligen enough for this at this point, just figuring it on-the-go. I may work on that if I ever feel comfortable enough with the code (tbh, I'd have no idea where this variable should even go).

@c-blake
Copy link
Owner

c-blake commented May 27, 2021

By default the parser accepts (any unique prefix) of the names. You are right that it is entirely possible that "in a CLI", especially in a terminal column-constrained setting, that you may want to only list the unique prefixes instead of exposing the full internal name. But that is a judgement call..maybe a bit more of the prefix than strictly the shortest unique. "red" instead of "r" for short things, but maybe "foo" instead of "fooBarBaz" or instead of "f" for long things. What is "descriptive enough" is a subjective judgement and means you will probably edit the help[enum key] string anyway. Which means the feature idea doesn't help.

So, it's not the strongest feature idea ever. But it's also not awful, as a "better default" for the lazy. I've always been of somewhat "mixed minds" about whether to even allow CLauthors to be so lazy as to inflict default help messages on CLusers. It's nice to be low effort/incremental, but in any long-term sense, the name of a parameter only goes so far. So, where it really matters you want that manual help, but then it doesn't always really matter. Hence, mixed minds. :-)

@c-blake
Copy link
Owner

c-blake commented May 27, 2021

The right answer to the mixed minds, btw, might be a warning message if help is totally empty indicating zero CLauthor attention given. I tend to shy away from merely informational warning messages as I've often disliked them in C/C++ compiler output as a "weak lint", though. So, I have just stick with advising CLauthors to be kind to --help runners.

@c-blake
Copy link
Owner

c-blake commented May 27, 2021

I should also have said, since you may be new-ish to Nim, that you can adjust the "string version" of the enum values to your liking and the cligen/argcvt.nim uses the string version in its parsing. So, you can do like the first line in test/Enums.nim to make the program identifier in an API call very different from the $value string representation of the enum value..just in case that is what you meant by "exposing internal names".

AFAIK, there is no way to do this "after the fact"..only in the enum definition, though. So, bolting a CLI on top of an existing API might not be so smooth in terms of this feature. One could probably do a macro to define a new enum type with new strings and then conversion procs between the two types and then also a wrapper proc that cligen.dispatch then actually wraps. That's pretty involved to just re-do guess the way enum labels are spelled, though.

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented May 27, 2021

I should also have said, since you may be new-ish to Nim, that you can adjust the "string version" of the enum values to your liking and the cligen/argcvt.nim uses the string version in its parsing. So, you can do like the first line in test/Enums.nim to make the program identifier in an API call very different from the $value string representation of the enum value..just in case that is what you meant by "exposing internal names".

You can see I already know that from the mock-up in original post here, where I explicitly set a short string values for enums to show to the user in help (thus somewhat addressing the narrowness of the column).

By "exposing" I meant showing the enum type name in the clValType column.

AFAIK, there is no way to do this "after the fact"..only in the enum definition, though. So, bolting a CLI on top of an existing API might not be so smooth in terms of this feature. One could probably do a macro to define a new enum type with new strings and then conversion procs between the two types and then also a wrapper proc that cligen.dispatch then actually wraps. That's pretty involved to just re-do guess the way enum labels are spelled, though.

Let's not get carried away, I won't be able to keep up anyway ;) The only idea of the FR is to replace showing the type name (mostly useless to the user) with the list of the available values which the option takes. That's probably two issues mashed up, but too much of a bikeshedding even considered together.

@c-blake
Copy link
Owner

c-blake commented May 27, 2021

Oops. So, you did set strings. Sorry. My mistake. No offense intended.

Anyway, if a CLauthor hates the type name there & wants to make it something more CLuser friendly, they can just write your own override for cligen/argcvt:arg(Parse|Help)[T: enum] between import cligen & your dispatch. Even separate overloads for every enum type in the signature if you want. Like, e.g. test/FancyRepeats2.nim.

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