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

Display possible value for enum in Help text #92

Closed
jerriep opened this issue Apr 25, 2018 · 13 comments · Fixed by #276
Closed

Display possible value for enum in Help text #92

jerriep opened this issue Apr 25, 2018 · 13 comments · Fixed by #276
Assignees
Milestone

Comments

@jerriep
Copy link
Contributor

jerriep commented Apr 25, 2018

I have several options for my command which are limited to enumerated types. Currently, the help text is displayed as follows:

Usage: ghi list [options]

Options:
  -?|-h|--help            Show help information
  -a|--all                All
  -r|--repo <REPOSITORY>  The repository to limit the issues to
  -u|--user <USER>        The user who the issues are related to
  -R|--rel <RELATION>     The relation of the issues to the user
  -s|--state <STATE>      The state of the issues
  -t|--token <TOKEN>      Your GitHub Personal Access token

But I want to display the possible values for the enumerated types. What I have done (for now) is to update the rel option, for example, as follows:

[Option(CommandOptionType.SingleValue, Description = "The relation of the issues to the user", ShortName = "R", LongName = "rel",
    ValueName = "Assigned|Created|Mentioned")]
public IssueRelation Relation { get; set; } = IssueRelation.Assigned;

This will then display the possible values as follows:

Usage: ghi list [options]

Options:
  -?|-h|--help                           Show help information
  -a|--all                               All
  -r|--repo <REPOSITORY>                 The repository to limit the issues to
  -u|--user <USER>                       The user who the issues are related to
  -R|--rel <Assigned|Created|Mentioned>  The relation of the issues to the user
  -s|--state <STATE>                     The state of the issues
  -t|--token <TOKEN>                     Your GitHub Personal Access token

Is this the correct way to go about it? Or is there a way to automatically display the list of values in the case of enumerated types which I have missed?

Also, is this not perhaps something we can add as a standard "feature" to the generated help text? I think it is pretty common for certain options to be limited to specific predefined values.

@natemcmaster natemcmaster added this to the Future milestone Apr 25, 2018
@natemcmaster
Copy link
Owner

This is a great question. I've had similar thoughts about trying to use validators or type inference to generate help text. I've avoided doing this so far as it can be complicated to do it automatically. What you've done for now is what I've been recommending to users as the simplest fix.

I would be open to investigating this more. My current thinking is that we could update the DefaultHelpTextGenerator to inspect the T in Option<T> and see if it can generate useful suggestions from its type.

@jerriep
Copy link
Contributor Author

jerriep commented Apr 25, 2018

I have subsequently changed it to actually display the list of possible values in the description, e.g.

Usage: ghi list [options]

Options:
  -?|-h|--help             Show help information
  -a|--all                 All
  -r|--repo <REPOSITORY>   The repository to limit the issues to
  -u|--user <USER>         The user who the issues are related to
  -R|--rel <RELATION>      The relation of the issues to the user (Assigned|Created|Mentioned)
  -s|--state <STATE>       The state of the issues
  -t|--token <TOKEN>       Your GitHub Personal Access token

I actually like that more, because otherwise the left-hand column of that option table may get too wide. So on second thought, perhaps this is better if you leave it to the developer to manually display however they choose?

I won't mind if you close this issue.

@natemcmaster
Copy link
Owner

I've had similar requests, so not going to close. I think its a useful feature; I just haven't thought thru an API for it.

@atruskie
Copy link
Contributor

I currently do this manually:

  -l|--log-level <LOG_LEVEL>  Set the log vebosity level. Valid values: None = 0, Error = 1, Warn = 2, Info = 3, Debug = 4, Trace = 5, Verbose = 6, All = 7

I'd be interested in this feature as well.

My format (of listing integer values as well) is useful but I'd estimate that it is generally not what people would want.

@jerriep
Copy link
Contributor Author

jerriep commented Apr 26, 2018

Another thing is that it may also be necessary to indicate the default option, So as per @atruskie sample above, maybe something like this:

-l|--log-level <LOG_LEVEL>  Set the log vebosity level. Valid values: None = 0, Error = 1, Warn = 2, Info = 3, Debug = 4 (default), Trace = 5, Verbose = 6, All = 7

@natemcmaster
Copy link
Owner

Whatever API we come up with needs to be flexible enough to allow users to control the output. I don't want to add behavior that is hard to override or change. While some may like the int<->Enum mapping, I know others who would not.

@bjorg
Copy link
Contributor

bjorg commented Jul 1, 2018

I filed a question about SingleOrNoValue options, which relates to this (#113).

Here's my implementation for parsing an enum option in case someone finds it useful. The following code parses the optional suffix either as int or string. In addition, it checks that the parsed value is valid. If not, it shows a description with the allowed values.

bool TryParseEnumOption<T>(
    CommandOption option, 
    T defaultvalue, 
    out T result
) where T : struct {
    if(option.Value() == null) {
        result = defaultvalue;
        return true;
    }
    if(int.TryParse(option.Value(), out int intValue)) {
        if(!Enum.GetValues(typeof(T)).Cast<int>().Any(v => v == intValue)) {
            goto failed;
        }
        result = (T)Convert.ChangeType(Enum.ToObject(typeof(T), intValue), typeof(T));
        return true;
    }
    if(Enum.TryParse(typeof(T), option.Value(), ignoreCase: true, result: out object enumValue)) {
        result = (T)Convert.ChangeType(enumValue, typeof(T));
        return true;
    }
failed:
    var pairs = Enum.GetValues(typeof(T))
        .Cast<int>()
        .Zip(
            Enum.GetNames(typeof(T)).Cast<string>(), 
            (value, name) => $"{value}={name.ToLowerInvariant()}"
        );
    Console.WriteLine($"value for {option.Template} must be one of {string.Join(", ", pairs)}");
    result = defaultvalue;
    return false;
}

Intended usage is:

if(verboseLevelOption.HasValue()) {
    if(!TryParseEnumOption(
        verboseLevelOption, 
        VerboseLevel.Normal, 
        out _verboseLevel
    )) {

        // handle option error
        return;
    }
}

It doesn't handle the default case, although it seems that would be a simple addition since it already has everything otherwise.

@natemcmaster
Copy link
Owner

I've put this in the 2.3.0 milestone. I'm planning to implement API to control help text in a more granular way, without having to re-implement all of IHelpTextGenerator. Let me know if you are interested in helping implement it.

@natemcmaster natemcmaster modified the milestones: Future, 2.3.0 Jul 5, 2018
@natemcmaster natemcmaster added the help wanted We would be willing to take a well-written PR to help fix this. label Dec 4, 2018
@natemcmaster natemcmaster modified the milestones: 2.3.0, 3.0.0 Dec 4, 2018
@fugaku
Copy link

fugaku commented Dec 13, 2018

Is this still in the 2.3.0 milestone ?

@natemcmaster
Copy link
Owner

No. I moved it out because I'm not planning to implement this myself. If you'd like to take this one, let me know.

@natemcmaster natemcmaster removed this from the 3.0.0 milestone Jan 6, 2019
@stale
Copy link

stale bot commented Apr 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 7 days.

@stale stale bot added the closed-wontfix This issue is closed because there are no plans to fix this. label Apr 6, 2019
@stale stale bot closed this as completed Apr 13, 2019
@natemcmaster natemcmaster added closed-stale This issue is closed because it went stale and there was no action planned. It can be reopened. and removed closed-wontfix This issue is closed because there are no plans to fix this. labels May 17, 2019
@kbilsted
Copy link
Contributor

kbilsted commented Sep 6, 2019

Would you be interested in an implementation that simply prints out the legal values.

existing behaviour

Options:
  -v|--verbosity <VERBOSITY>                     Set verbosity level
  -NoRegions|--remove-regions                    Remove #region
  -RC|--remove-custom-tags <REMOVE_CUSTOM_TAGS>  Remove custom <tag>'s'
  -?|-h|--help                                   Show help information

> dotnet .\myprogram.dll -v h              Invalid value specified for verbosity. Allowed values are: None, ShowOnlyChangedFiles, ShowAllFiles.

then becomes

Options:
  -v|--verbosity <VERBOSITY>                     Set verbosity level 
                                                 Allowed values are None, ShowOnlyChangedFiles, ShowAllFiles
                                                 
  -NoRegions|--remove-regions                    Remove #region
  -RC|--remove-custom-tags <REMOVE_CUSTOM_TAGS>  Remove custom <tag>'s'
  -?|-h|--help                                   Show help information

@natemcmaster natemcmaster reopened this Sep 14, 2019
@stale stale bot removed the closed-stale This issue is closed because it went stale and there was no action planned. It can be reopened. label Sep 14, 2019
@natemcmaster
Copy link
Owner

@kbilsted yes!

kbilsted added a commit to kbilsted/CommandLineUtils that referenced this issue Sep 14, 2019
kbilsted added a commit to kbilsted/CommandLineUtils that referenced this issue Sep 14, 2019
When options or arguments are parsed into an enum, the help text will now include the values contained in the enum.

fixes natemcmaster#92
@natemcmaster natemcmaster removed the help wanted We would be willing to take a well-written PR to help fix this. label Sep 15, 2019
@natemcmaster natemcmaster added this to the 2.5.0 milestone Sep 15, 2019
kbilsted added a commit to kbilsted/CommandLineUtils that referenced this issue Sep 19, 2019
When options or arguments are parsed into an enum, the help text will now include the values contained in the enum.

fixes natemcmaster#92
natemcmaster pushed a commit that referenced this issue Sep 20, 2019
When options or arguments are parsed into an enum, the help text will now include the values contained in the enum.

Fixes #92

Also made the default constructor public. This allows the DefaultHelpTextGenerator to be instantiated on a per-use basis in order to keep unit tests a isolated from each other as possible.
Without that change, all instances would be the same singleton instance - i.e. a test can affect another test by fiddling with the overridden width.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants