DevSkim icon indicating copy to clipboard operation
DevSkim copied to clipboard

Confusing help text for `-g`/`--ignore-globs` option

Open agr opened this issue 2 years ago • 7 comments

Describe the bug Running devskim analyze --help produces this line:

-g, --ignore-globs (Default: /.git/ /bin/) Comma-separated Globs for files to skip analyzing

Notice, the text says "Comma-separated", but the default value is space-separated. So, which is it?

To Reproduce Steps to reproduce the behavior:

  1. Run devskim analyze --help
  2. Scan the output for the line mentioned above.

Expected behavior Displayed defaults/examples should be consistent with description text.

Screenshots image

Versions(please complete the following information):

  • OS: Windows 10 22H2 / 19045.3086
  • Devskim Version 1.0.11+87ad45b866

agr avatar Jul 07 '23 22:07 agr

Also, help text has whole bunch of extra empty lines, so the output unnecessarily doesn't fit into one screen and I have to scroll up to see it in its entirety.

agr avatar Jul 07 '23 22:07 agr

@agr

Sorry for the confusion. The correct way to provide the arguments on the command line is comma separated. This gets automatically split on commas by the CommandLineParser dependency, giving the application itself an IEnumerable<string>, with the strings subsequently converted to Globs for pattern matching.

You can see here where the globs argument is defined: https://github.com/microsoft/DevSkim/blob/87ad45b8661cc8ecc22fcc1e386c969b8a01cf36/DevSkim-DotNet/Microsoft.DevSkim.CLI/Options/BaseAnalyzeCommandOptions.cs#L45

I believe the help text displays this as space separated because the actual default value is an IEnumerable as if it had already been parsed.

We can investigate if it possible to change how this is represented to the user, as I agree it may be confusing that the spaces are not rendered as part of the default.

As for the empty lines, do you see something like this, with one line break between each argument or are you seeing multiple line breaks between each argument? We can also look into if its possible to remove those line breaks between argument specifications, as there are a large number of arguments so condensing it may be helpful.

image

gfs avatar Jul 07 '23 22:07 gfs

Yes, that's exactly what I am talking about. When I am experimenting with options and mess up so devskim doesn't actually do anything, it produces an error, then follows it with the help screen which overflows the output and I have to scroll up to see what went wrong. Just try running devskim analyze as an example. Alternative here would be not to print help on failure maybe?

agr avatar Jul 07 '23 23:07 agr

It looks like removing the extra newlines is a supported configuration change, but it requires a bit of refactoring as we currently use the default parser (https://github.com/commandlineparser/commandline/wiki/HelpText-Configuration).

Its not clear to me that it would be easy to change the default values to express the comma separator, but we can investigate that as well.

@daalcant

gfs avatar Jul 08 '23 00:07 gfs

None of it is a big deal, just an inconvenience, so I don't expect any priority for that. And should the missing comma in defaults be actually reported to CommandLineParser?

agr avatar Jul 10 '23 22:07 agr

@agr Thanks for your understanding.

As for the missing commas, that does seem to me like an oversight in the default help generator from command line parser.

I didn't immediately see any configurable option to address that - unlike removing the extra line - if there isn't one it's likely something that will need to addressed there first.

gfs avatar Jul 11 '23 01:07 gfs

It looks like the separator missing from the Default value is already a reported issue in the CommandLine repo: https://github.com/commandlineparser/commandline/issues/490

I opened a PR to resolve it: https://github.com/commandlineparser/commandline/pull/895

gfs avatar Jul 27 '23 23:07 gfs