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

move verbose setting parsing to CLI #88

Closed
wants to merge 1 commit into from

Conversation

adamralph
Copy link
Owner

Relates to #3.

@adamralph adamralph added this to the 1.0.0 milestone Nov 5, 2018
@bording
Copy link
Collaborator

bording commented Nov 5, 2018

An alternative to this would be to keep things as is, but instead of caring specifically about true or false values specifically, you just treat anything that's not true to be false.

That's a fairly common way of doing things in MSBuild.

@adamralph
Copy link
Owner Author

@bording I did consider that, but I'm not sure it's a good idea equate fat-fingered values like "tru", "ture", "true ", etc .to false. I think it's better to surface the problem as early as possible.

@bording
Copy link
Collaborator

bording commented Nov 5, 2018

If this was a more critical value, then maybe, but the side effect of setting this is incorrectly is less verbose logging.

@adamralph
Copy link
Owner Author

That's true, but the user may not realise what they've done and get confused, or even report a bug. It would be swallowing a user's mistake, and I'm not sure that's a good idea.

@adamralph
Copy link
Owner Author

adamralph commented Nov 5, 2018

I've decided against it anyway. The error message goes from this:

C:\Users\adam\.nuget\packages\minver\1.0.0-alpha.13\build\MinVer.targets(17,5): error MINVER0003: MinVerVerbose/MINVER_VERBOSE value 'foo' cannot be converted to a Boolean value. [C:\code\adamralph\liteguard\src\LiteGuard\LiteGuard.csproj]

to this:

  Unhandled Exception: McMaster.Extensions.CommandLineUtils.CommandParsingException: Invalid value specified for verbose. Cannot convert 'foo' to a boolean. ---> System.FormatException: Invalid value specified for verbose. Cannot convert 'foo' to a boolean.
     at McMaster.Extensions.CommandLineUtils.Abstractions.BooleanValueParser.Parse(String argName, String value, CultureInfo culture) in C:\projects\commandlineutils\src\CommandLineUtils\Internal\ValueParsers\BooleanValueParser.cs:line 33
     at McMaster.Extensions.CommandLineUtils.CommandOption`1.McMaster.Extensions.CommandLineUtils.IInternalCommandParamOfT.Parse(CultureInfo culture) in C:\projects\commandlineutils\src\CommandLineUtils\CommandOption{T}.cs:line 47
     at McMaster.Extensions.CommandLineUtils.CommandLineApplication.HandleParseResult(ParseResult parseResult) in C:\projects\commandlineutils\src\CommandLineUtils\CommandLineApplication.cs:line 548
     --- End of inner exception stack trace ---
     at McMaster.Extensions.CommandLineUtils.CommandLineApplication.HandleParseResult(ParseResult parseResult) in C:\projects\commandlineutils\src\CommandLineUtils\CommandLineApplication.cs:line 574
     at McMaster.Extensions.CommandLineUtils.CommandLineApplication.Parse(String[] args) in C:\projects\commandlineutils\src\CommandLineUtils\CommandLineApplication.cs:line 535
     at McMaster.Extensions.CommandLineUtils.CommandLineApplication.Execute(String[] args) in C:\projects\commandlineutils\src\CommandLineUtils\CommandLineApplication.cs:line 597
     at MinVer.Cli.Program.Main(String[] args) in C:\code\adamralph\min-ver\MinVer.Cli\Program.cs:line 51
C:\Users\adam\AppData\Local\Temp\min-ver-tests-package\packages\minver\1.0.0-build.0\build\MinVer.targets(15,5): error MSB3073: The command "dotnet "C:\Users\adam\AppData\Local\Temp\min-ver-tests-package\packages\minver\1.0.0-build.0\build\../MinVer/MinVer.Cli.dll" --build-metadata "build.1" --path "C:\Users\adam\AppData\Local\Temp\min-ver-tests-package" --major-minor "" --tag-prefix "v." --verbose "foo"" exited with code -532462766. [C:\Users\adam\AppData\Local\Temp\min-ver-tests-package\min-ver-tests-package.csproj]

@adamralph adamralph closed this Nov 5, 2018
@adamralph adamralph deleted the verbose-setting-parsing branch November 5, 2018 22:45
@adamralph adamralph added the wontfix This will not be worked on label Nov 5, 2018
@adamralph adamralph removed this from the 1.0.0 milestone Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants