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

GH4278: Update Spectre.Console to 0.48.0 #4279

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

devlead
Copy link
Member

@devlead devlead commented Nov 24, 2023

@devlead
Copy link
Member Author

devlead commented Nov 24, 2023

It would seem Spectre.Console 0.48.0 has a breaking change with settings like

[CommandOption("--version|--ver")]
[Description("Displays version information.")]
public bool ShowVersion { get; set; }

This test fails

[Theory]
[InlineData("--version")]
[InlineData("--ver")]
public async Task The_Version_Option_Should_Call_Version_Feature(params string[] args)

only the last --ver works.

image

not investigated the root cause fully.

@devlead
Copy link
Member Author

devlead commented Nov 24, 2023

Found the issue Spectre.Console.Cli no longer allows default commands to have a --version parameter, which Cake relied on to have it's own custom version command.

spectreconsole/spectre.console@131b37f#diff-d38c9fa48d9c39f8eec58957b2333d13943f2cbe1adee26f30325e527a56f600L26-R41

Will see if I can find away around that, can't use the default Spectre version command as it will produce a version string that will break Cake cli contract. Overriding command settings ApplicationVersion and removing cake version feature should probably achieve the same result for end user.

@devlead
Copy link
Member Author

devlead commented Nov 24, 2023

As it's by design breaking change in Spectre.Console.Cli, I just added a workaround to opt-out of this behavior for Cake.Tool.

"-v" => "--ver",
"--version" => "--ver",
_ => arg
}));
Copy link
Member Author

@devlead devlead Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a terrible workaround, should've probably just addressed if first arg is -v / --version... but the problem goes deeper.

Spectre.Console.Cli switched from

-                if (firstArgument.Equals("--version", StringComparison.OrdinalIgnoreCase) ||
-                    firstArgument.Equals("-v", StringComparison.OrdinalIgnoreCase))

to

+        if (args.Contains("-v") || args.Contains("--version"))
+        {

so the default command behavior changed from first to any supplied argument is --version / -v will print the version and exit the application. even i.e. -- between command and remaining args used.

so will have to think about this a bit more. moving PR to WIP.

@devlead devlead marked this pull request as draft November 24, 2023 20:35
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

Successfully merging this pull request may close these issues.

Update Spectre.Console to 0.48.0
1 participant