Skip to content

Honor user defined order of commands, subcommands and options#1215

Closed
gabrielmaldi wants to merge 1 commit intodotnet:mainfrom
gabrielmaldi:help-items-defined-order
Closed

Honor user defined order of commands, subcommands and options#1215
gabrielmaldi wants to merge 1 commit intodotnet:mainfrom
gabrielmaldi:help-items-defined-order

Conversation

@gabrielmaldi
Copy link
Copy Markdown

@gabrielmaldi gabrielmaldi commented Feb 26, 2021

Currently, HelpBuilder sorts help items, which results in the following behavior:

Example 1

new CommandEx(new[] { "migrations", "mig" })
Commands:
  mig, migrations    Commands to manage migrations.

Example 2

new CommandEx(new[] { "health-checks", "hc" })
new CommandEx(new[] { "mini-profiler", "mp" })
mini-profiler, mp                Generates the MiniProfiler script
hc, health-checks <from> <to>    Generates the HealthChecks script [from: '0' (the initial database), to: the last migration]

User defined order should be respected; otherwise, the aliases (hc) can be shown before the main command (health-checks). Also, multiple commands can result in odd ordering: mini-profiler, mp and hc, health-checks (main/alias, alias/main; see Example 2).


For completeness:

public CommandEx(string[] aliases, string description)
    : base(aliases.First(), description)
{
    foreach (var alias in aliases.Skip(1))
    {
        AddAlias(alias);
    }
}

@gabrielmaldi gabrielmaldi force-pushed the help-items-defined-order branch from 781746e to 9eb1129 Compare February 26, 2021 20:00
@dnfadmin
Copy link
Copy Markdown

dnfadmin commented Feb 26, 2021

CLA assistant check
All CLA requirements met.

@gabrielmaldi gabrielmaldi force-pushed the help-items-defined-order branch from 9eb1129 to fd49a4e Compare February 26, 2021 20:07
@jonsequitur
Copy link
Copy Markdown
Contributor

The convention in most command line help (at least for options) is for the short-form aliases to precede the long-form aliases, so the current ordering is deliberate.

There's been discussion though about making help easier to customize, in a way that would accommodate the desire to reorder the aliases.

@Keboo @adamhewitt627 ...thoughts?

@Keboo
Copy link
Copy Markdown
Member

Keboo commented Feb 28, 2021

I agree, I think this is another case that needs to be handled with making configuring the help output

@gabrielmaldi
Copy link
Copy Markdown
Author

gabrielmaldi commented Feb 28, 2021

This is how I'm handling it right now:

new CommandLineBuilder(rootCommand)
    .UseHelpBuilder(context => new HelpBuilderEx(context.Console))
    .UseDefaults()
    .Build()
    .Invoke(args);
public class HelpBuilderEx : HelpBuilder
{
    public HelpBuilderEx(
        IConsole console,
        int? columnGutter = null,
        int? indentationSize = null,
        int? maxWidth = null)
        : base(console, columnGutter, indentationSize, maxWidth)
    {
    }

    protected override string DefaultValueHint(IArgument argument, bool isSingleArgument)
    {
        if (argument is IArgumentEx argumentEx && argumentEx.DefaultValueHintFactory != null)
        {
            var surrogate = new Argument(argument.Name);
            surrogate.SetDefaultValueFactory(argumentEx.DefaultValueHintFactory);
            argument = surrogate;
        }

        return base.DefaultValueHint(argument, isSingleArgument);
    }
}

Which of course is less than ideal. I agree that it would be better to make HelpBuilder more configurable instead of just removing the call to ThenBy. In that regard, I would change this method to protected:

private IEnumerable<HelpItem> GetOptionHelpItems(IIdentifierSymbol symbol)
{
    var rawAliases = symbol
                     .Aliases
                     .Select(r => r.SplitPrefix())
                     .OrderBy(r => r.prefix, StringComparer.OrdinalIgnoreCase)
                     .GroupBy(t => t.alias)
                     .Select(t => t.First())
                     .Select(t => $"{t.prefix}{t.alias}");

And extract this bit (rawAliases) into a new protected method so it can be overridden. I would be happy to submit a new PR and close this one.

@KalleOlaviNiemitalo
Copy link
Copy Markdown

Is this needed after #2073?

CliCommand and CliOption now have both public string Name { get; } and public ICollection<string> Aliases { get; }, and help displays Name before Aliases:

var aliases = aliasSet is null
? new [] { symbol.Name }
: new [] {symbol.Name}.Concat(aliasSet)
.Select(r => r.SplitPrefix())
.OrderBy(r => r.Prefix, StringComparer.OrdinalIgnoreCase)
.ThenBy(r => r.Alias, StringComparer.OrdinalIgnoreCase)
.GroupBy(t => t.Alias)
.Select(t => t.First())
.Select(t => $"{t.Prefix}{t.Alias}");

Even if you still need to preserve the order of aliases, you cannot do that just by making HelpBuilder sort the aliases, because AliasSet stores them to HashSet<string> _aliases, which already does not preserve order.

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.

5 participants