commandline icon indicating copy to clipboard operation
commandline copied to clipboard

Required Value treated as option

Open carstencodes opened this issue 4 years ago • 2 comments

Synopsis If a required value with a MetaName is omitted, the error is depicted as Option, which might be misleading:

A required value not bound to option name is missing.

Version used

2.8.0

System Description

  • Windows 10 (.NET FullFramework 4.8)
  • Ubuntu Linux 18.04 (.NET Core 3.1)
  • Target Assembly Compiled as: .NET Standard 2.0

Gist for reproduction Example Repository

In this gist, two verbs are introduced both with a required positional argument. Both Values have a MetaName. The meta name is used in the description during the help text, but not in the error message.


Actual behavior

$ calc fib --help
calc 1.0.0
Copyright (C) 2021 calc

  --help                                                    Display this help screen.

  --version                                                 Display version information.

  position of the fibonacci number to calculate (pos. 0)    Required.

This is ok and expected. But omitting the --help leads to the following error:

$ calc fib
calc 1.0.0
Copyright (C) 2021 calc

ERROR(S):
  A required value not bound to option name is missing.

  --help                                                    Display this help screen.

  --version                                                 Display version information.

  position of the fibonacci number to calculate (pos. 0)    Required.

Expected behavior

$ calc fib
calc 1.0.0
Copyright (C) 2021 calc

ERROR(S):
  The required position of the fibonacci number to calculate (value at position 0) is missing.

  --help                                                    Display this help screen.

  --version                                                 Display version information.

  position of the fibonacci number to calculate (pos. 0)    Required.

Possible error sources analyzed

In SpecificationPropertyRules, a MissingRequiredOptionError error is issued for the ValueSpecification using the NameExtensions FromSpecification method. For a ValueSpecification, the resulting NameInfo object is set to an empty instance. This might be intentional, but will cause the error here.


Suggested fixes

  1. Extend NameExtensions class as follows:
       public static NameInfo FromValueSpecification(this ValueSpecification specification)
        {
            return !string.IsNullOrWhitespace(specification.MetaName)
              ? new NameInfo(
                string.Empty,
                specification.MetaName)
             : NameInfo.EmptyName ;
        }

        public static NameInfo FromSpecification(this Specification specification)
        {
            switch (specification.Tag)
            {
                case SpecificationType.Option:
                    return FromOptionSpecification((OptionSpecification)specification);
                case SpecificationType.Value:
                    return FromValueSpecification((ValueSpecification)specification);
                default:
                    return NameInfo.EmptyName;
            }
        }

This clarifies the error message more to

ERROR(S):
  ​Required option 'position of the fibonacci number to calculate' is missing.
  1. If this is not yet appropriate enough, we can update SpecificationPropertyRules in addition to 1 like follows:
from sp in missing
                    select
                        sp.IsOption() ? new MissingRequiredOptionError(sp.FromSpecification()) as Error
                                      : new MissingValueOptionError(sp.FromSpecification()) as Error;

This changes the error to

Option 'position of the fibonacci number to calculate' has no value.

which might not be good.

  1. Take approach (2) but introduce a new MissingRequiredPositionalValueError including the extension of Errors, SentenceBuilder, etc.

If you accept the issue, I can submit a PullRequest for each of the three issues, if you tell me, which way to go.

If you do not accept the issue, please tell me a feasible workaround or tell me the reason, why this should be considered as intentional.

Cheers

Carsten

carstencodes avatar May 04 '21 13:05 carstencodes

Running into this exact issue

jacksoncougar avatar May 04 '21 15:05 jacksoncougar

Me too - somewhere I found that in theory using preview 3 with GetOptMode = true would fix it, but the lack of availabilty on nuget.org had me struggling.

rpavlik avatar Jun 11 '21 20:06 rpavlik