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

Help output correctly decides when to show the version option #1664

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Spectre.Console.Cli/CommandParseException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,9 @@ internal static CommandParseException ValueIsNotInValidFormat(string value)
var text = $"[red]Error:[/] The value '[white]{value}[/]' is not in a correct format";
return new CommandParseException("Could not parse value", new Markup(text));
}

internal static CommandParseException UnknownParsingError()
{
return new CommandParseException("An unknown error occured when parsing the arguments.");
}
}
27 changes: 19 additions & 8 deletions src/Spectre.Console.Cli/Help/HelpProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,27 @@ public static IReadOnlyList<HelpOption> Get(
new HelpOption("h", "help", null, null, resources.PrintHelpDescription, null),
};

// Version information applies to the entire application
// Include the "-v" option in the help when at the root of the command line application
// Don't allow the "-v" option if users have specified one or more sub-commands
if ((command?.Parent == null) && !(command?.IsBranch ?? false))
// Version information applies to the entire CLI application.
// Whether to show the "-v|--version" option in the help is determined as per:
// - If an application version has been set, and
// -- When at the root of the application, or
// -- When at the root of the application with a default command, unless
// --- The default command has a version option in its settings
if ((command?.Parent == null) && !(command?.IsBranch ?? false) && (command?.IsDefaultCommand ?? true))
{
// Only show the version command if there is an
// application version set.
if (model.ApplicationVersion != null)
// Check whether the default command has a version option in its settings.
var versionCommandOption = command?.Parameters?.OfType<CommandOption>()?.FirstOrDefault(o =>
(o.ShortNames.FirstOrDefault(v => v.Equals("v", StringComparison.OrdinalIgnoreCase)) != null) ||
(o.LongNames.FirstOrDefault(v => v.Equals("version", StringComparison.OrdinalIgnoreCase)) != null));

// Only show the version option if the default command doesn't have a version option in its settings.
if (versionCommandOption == null)
{
parameters.Add(new HelpOption("v", "version", null, null, resources.PrintVersionDescription, null));
// Only show the version option if there is an application version set.
if (model.ApplicationVersion != null)
{
parameters.Add(new HelpOption("v", "version", null, null, resources.PrintVersionDescription, null));
}
}
}

Expand Down
136 changes: 111 additions & 25 deletions src/Spectre.Console.Cli/Internal/CommandExecutor.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using static Spectre.Console.Cli.CommandTreeTokenizer;

namespace Spectre.Console.Cli;

internal sealed class CommandExecutor
Expand All @@ -12,6 +14,8 @@ public CommandExecutor(ITypeRegistrar registrar)

public async Task<int> Execute(IConfiguration configuration, IEnumerable<string> args)
{
CommandTreeParserResult parsedResult;

if (configuration == null)
{
throw new ArgumentNullException(nameof(configuration));
Expand All @@ -27,20 +31,37 @@ public async Task<int> Execute(IConfiguration configuration, IEnumerable<string>
_registrar.RegisterInstance(typeof(CommandModel), model);
_registrar.RegisterDependencies(model);

// No default command?
if (model.DefaultCommand == null)
// Got at least one argument?
var firstArgument = arguments.FirstOrDefault();
if (firstArgument != null)
{
// Got at least one argument?
var firstArgument = arguments.FirstOrDefault();
if (firstArgument != null)
// Asking for version?
if (firstArgument.Equals("-v", StringComparison.OrdinalIgnoreCase) ||
firstArgument.Equals("--version", StringComparison.OrdinalIgnoreCase))
{
// Asking for version? Kind of a hack, but it's alright.
// We should probably make this a bit better in the future.
if (firstArgument.Equals("--version", StringComparison.OrdinalIgnoreCase) ||
firstArgument.Equals("-v", StringComparison.OrdinalIgnoreCase))
if (configuration.Settings.ApplicationVersion != null)
{
if (configuration.Settings.ApplicationVersion != null)
// We need to check if the command has a version option on its setting class.
// Do this by first parsing the command line args and checking the remaining args.
try
{
// Parse and map the model against the arguments.
parsedResult = ParseCommandLineArguments(model, configuration.Settings, arguments);
}
catch (Exception)
{
// Something went wrong with parsing the command line arguments,
// however we know the first argument is a version option.
var console = configuration.Settings.Console.GetConsole();
console.MarkupLine(configuration.Settings.ApplicationVersion);
return 0;
}

// Check the parsed remaining args for the version options.
if ((firstArgument.Equals("-v", StringComparison.OrdinalIgnoreCase) && parsedResult.Remaining.Parsed.Contains("-v")) ||
(firstArgument.Equals("--version", StringComparison.OrdinalIgnoreCase) && parsedResult.Remaining.Parsed.Contains("--version")))
{
// The version option is not a member of the command settings.
var console = configuration.Settings.Console.GetConsole();
console.MarkupLine(configuration.Settings.ApplicationVersion);
return 0;
Expand All @@ -50,7 +71,7 @@ public async Task<int> Execute(IConfiguration configuration, IEnumerable<string>
}

// Parse and map the model against the arguments.
var parsedResult = ParseCommandLineArguments(model, configuration.Settings, arguments);
parsedResult = ParseCommandLineArguments(model, configuration.Settings, arguments);

// Register the arguments with the container.
_registrar.RegisterInstance(typeof(CommandTreeParserResult), parsedResult);
Expand Down Expand Up @@ -101,34 +122,99 @@ public async Task<int> Execute(IConfiguration configuration, IEnumerable<string>
}
}

[SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1513:Closing brace should be followed by blank line", Justification = "Improves code readability by grouping together related statements into a block")]
private CommandTreeParserResult ParseCommandLineArguments(CommandModel model, CommandAppSettings settings, IReadOnlyList<string> args)
{
var parser = new CommandTreeParser(model, settings.CaseSensitivity, settings.ParsingMode, settings.ConvertFlagsToRemainingArguments);
CommandTreeParserResult? parsedResult = null;
CommandTreeTokenizerResult tokenizerResult;

var parserContext = new CommandTreeParserContext(args, settings.ParsingMode);
var tokenizerResult = CommandTreeTokenizer.Tokenize(args);
var parsedResult = parser.Parse(parserContext, tokenizerResult);
try
{
(parsedResult, tokenizerResult) = InternalParseCommandLineArguments(model, settings, args);

var lastParsedLeaf = parsedResult.Tree?.GetLeafCommand();
var lastParsedCommand = lastParsedLeaf?.Command;

var lastParsedLeaf = parsedResult.Tree?.GetLeafCommand();
var lastParsedCommand = lastParsedLeaf?.Command;
if (lastParsedLeaf != null && lastParsedCommand != null &&
if (lastParsedLeaf != null && lastParsedCommand != null &&
lastParsedCommand.IsBranch && !lastParsedLeaf.ShowHelp &&
lastParsedCommand.DefaultCommand != null)
{
// Adjust for any parsed remaining arguments by
// inserting the the default command ahead of them.
var position = tokenizerResult.Tokens.Position;
foreach (var parsedRemaining in parsedResult.Remaining.Parsed)
{
position--;
position -= parsedRemaining.Count(value => value != null);
}
position = position < 0 ? 0 : position;

// Insert this branch's default command into the command line
// arguments and try again to see if it will parse.
var argsWithDefaultCommand = new List<string>(args);
argsWithDefaultCommand.Insert(position, lastParsedCommand.DefaultCommand.Name);

(parsedResult, tokenizerResult) = InternalParseCommandLineArguments(model, settings, argsWithDefaultCommand);
}
}
catch (CommandParseException) when (parsedResult == null && settings.ParsingMode == ParsingMode.Strict)
{
// Insert this branch's default command into the command line
// arguments and try again to see if it will parse.
var argsWithDefaultCommand = new List<string>(args);
// The parsing exception might be resolved by adding in the default command,
// but we can't know for sure. Take a brute force approach and try this for
// every position between the arguments.
for (int i = 0; i < args.Count; i++)
{
var argsWithDefaultCommand = new List<string>(args);
argsWithDefaultCommand.Insert(args.Count - i, "__default_command");

try
{
(parsedResult, tokenizerResult) = InternalParseCommandLineArguments(model, settings, argsWithDefaultCommand);

argsWithDefaultCommand.Insert(tokenizerResult.Tokens.Position, lastParsedCommand.DefaultCommand.Name);
break;
}
catch (CommandParseException)
{
// Continue.
}
}

if (parsedResult == null)
{
// Failed to parse having inserted the default command between each argument.
// Repeat the parsing of the original arguments to throw the correct exception.
InternalParseCommandLineArguments(model, settings, args);
}
}

parserContext = new CommandTreeParserContext(argsWithDefaultCommand, settings.ParsingMode);
tokenizerResult = CommandTreeTokenizer.Tokenize(argsWithDefaultCommand);
parsedResult = parser.Parse(parserContext, tokenizerResult);
if (parsedResult == null)
{
// The arguments failed to parse despite everything we tried above.
// Exceptions should be thrown above before ever getting this far,
// however the following is the ulimately backstop and avoids
// the compiler from complaining about returning null.
throw CommandParseException.UnknownParsingError();
}

return parsedResult;
}

/// <summary>
/// Parse the command line arguments using the specified <see cref="CommandModel"/> and <see cref="CommandAppSettings"/>,
/// returning the parser and tokenizer results.
/// </summary>
/// <returns>The parser and tokenizer results as a tuple.</returns>
private (CommandTreeParserResult ParserResult, CommandTreeTokenizerResult TokenizerResult) InternalParseCommandLineArguments(CommandModel model, CommandAppSettings settings, IReadOnlyList<string> args)
{
var parser = new CommandTreeParser(model, settings.CaseSensitivity, settings.ParsingMode, settings.ConvertFlagsToRemainingArguments);

var parserContext = new CommandTreeParserContext(args, settings.ParsingMode);
var tokenizerResult = CommandTreeTokenizer.Tokenize(args);
var parsedResult = parser.Parse(parserContext, tokenizerResult);

return (parsedResult, tokenizerResult);
}

private static async Task<int> Execute(
CommandTree leaf,
CommandTree tree,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ namespace Spectre.Console.Tests.Data;
public sealed class VersionSettings : CommandSettings
{
[CommandOption("-v|--version")]
[Description("The command version")]
public string Version { get; set; }
}
Loading