If Argument.HelpName was set, HelpBuilder.Default calls GetCompletions but ignores the result

[KalleOlaviNiemitalo]

HelpBuilder.Default.GetArgumentUsageLabel(Argument argument) calls argument.GetCompletions() unless argument.ValueType is bool nor bool?. However, if argument.HelpName is a non-empty string, then GetArgumentUsageLabel ignores the result of GetCompletions. In that situation, it would be better if GetArgumentUsageLabel skipped the GetCompletions call. Doing so would improve performance if the application has slow completion sources.

command-line-api/src/System.CommandLine/Help/HelpBuilder.Default.cs

Lines 66 to 92 in 6524142

	string firstColumn;
	var completions = argument
	.GetCompletions(CompletionContext.Empty)
	.Select(item => item.Label)
	.ToArray();
	var arg = argument;
	var helpName = arg?.HelpName ?? string.Empty;
	if (!string.IsNullOrEmpty(helpName))
	{
	firstColumn = helpName!;
	}
	else if (completions.Length > 0)
	{
	firstColumn = string.Join("|", completions);
	}
	else
	{
	firstColumn = argument.Name;
	}
	if (!string.IsNullOrWhiteSpace(firstColumn))
	{
	return $"<{firstColumn}>";
	}
	return firstColumn;

This scenario was not optimized in #1367 when HelpBuilder was changed to display Argument.HelpName.

Fixing this might solve dotnet/sdk#27374, in which .NET SDK uses a completion callback that ends up resolving MSBuild SDKs from NuGet, even though the completions won't be displayed in help. Such a slow completion callback could still be intolerable in dotnet-suggest, though.

[jonsequitur]

Under proposal #2071, another approach for turning off completions in help output that would be more explicit might look like this:

switch (parseResult.Action)
{
    case HelpAction helpAction:

        helpAction.UseCompletionsAsExamples = false;
        await helpAction.RunAsync();

        break;

    //...
}

Thoughts?

[KalleOlaviNiemitalo]

That looks like it would hide all completions from help, not provide the ability to hide the completions of one argument but keep the others.

[jonsequitur]

That's true. Does the existing implementation seem intuitive enough? Would deeper help configuration on the symbol objects themselves be more intuitive?

[KalleOlaviNiemitalo]

In my opinion, the way this works in 2.0 beta 4 is OK as an API, but:

• The useless work computing the completions should be avoided.
• The way to hide the completions should be documented in these places:
  • ArgumentHelpName itself.
  • How to customize help in apps that are built with the System.Commandline library, which does already mention ArgumentHelpName, but not for the purpose of hiding completions.
  • AddCompletions, although options with an enum type get completions even if the app does not call this method. I know this API has been changed after 2.0 beta 4.
  • CustomizeSymbol perhaps, on the assumption that a developer trying to hide the completions would be likely to look here.