dotnet
CompletionSources not sorted properly when building HelpName default suggestions
The sorting logic done here when building the default help text from the list of completions is backward:
https://github.com/dotnet/dotnet/blob/2db1f5ee2bdda2e8d873769325fabede32e420e0/src/command-line-api/src/System.CommandLine/Option.cs#L142
Since the ThenBy is on the Label, the resulting sorted collection is re-sorted by Label, as a primary order, not as a secondary order like SQL.
And, to hoist the current entry to the top of the list, the OrderBy needs to be OrderByDescending.
Here is an example that shows the bad behavior:
public static class Program
{
static CompletionItem[] sampleCompletions =
[
new ( "5", sortText: "05" ),
new ( "10", sortText: "10" ),
new ( "6", sortText: "06" ),
new ( "20", sortText: "10" ),
new ( "30", sortText: "30" )
];
public static async Task<int> Main ( string[] argv)
{
RootCommand root = new ( "example" );
Option<int> option = new ( "option" );
option.CompletionSources.Add ( static ctx => sampleCompletions );
root.Options.Add ( option );
await root.Parse ( [ "--help" ] ).InvokeAsync ( );
}
}
Output:
```text
Description:
example
Usage:
SnapsInAZfs [options]
Options:
option <10|20|30|5|6>
-?, -h, --help Show help and usage information
--version Show version information
Note the improperly ordered suggestions in the HelpName for option.
Here is why, isolated to what the line referenced at the top of the issue does:
CompletionContext theCompletionContext = CompletionContext.Empty;
IOrderedEnumerable<CompletionItem> improperlyOrdered = sampleCompletions
.OrderBy ( item => item.SortText.IndexOfCaseInsensitive ( theCompletionContext.WordToComplete ) )
.ThenBy ( item => item. Label, StringComparer.OrdinalIgnoreCase );
foreach ( CompletionItem c in improperlyOrdered )
{
Console.WriteLine ( c.Label );
}
Output:
10
20
30
5
6
Here is the fixed sorting code, as it applies to building HelpText (an empty context gets passed in):
IOrderedEnumerable<CompletionItem> properlyOrdered = sampleCompletions
.OrderByDescending ( item => item.Label.IndexOfCaseInsensitive ( theCompletionContext.WordToComplete ) )
.ThenBy ( item => item.SortText, StringComparer.OrdinalIgnoreCase );
foreach ( CompletionItem c in properlyOrdered )
{
Console.WriteLine ( c.Label );
}
Output:
5
6
10
20
30
And here is how it would look during tab completion, if the user had already entered "2" and then presses tab (simulated by giving it the "2" directly):
IOrderedEnumerable<CompletionItem> properlyOrderedWithPartialEntry = sampleCompletions
.OrderByDescending ( item => item.Label.IndexOfCaseInsensitive ( "2" ) )
.ThenBy ( item => item.SortText, StringComparer.OrdinalIgnoreCase );
foreach ( CompletionItem c in properlyOrderedWithPartialEntry )
{
Console.WriteLine ( c.Label );
}
Output:
20
5
6
10
30
~~The actual code to replace what is there now would be:~~
This is incorrect. See my follow-up comment that fixes it.
return completions
.OrderByDescending(item => item.Label.IndexOfCaseInsensitive(context.WordToComplete))
.ThenBy(symbol => symbol.SortText, StringComparer.OrdinalIgnoreCase);
Also, it may be worth pointing out that the extension method IndexOfCaseInsensitive does not use the same StringComparer as the code here. It uses invariant culture. This uses ordinal. So the two sorts aren't the same, necessarily, even if the values are identical.
Actually... That will sort reversed if there is no SortText...
Here is a solution to that:
return completions
.OrderByDescending( static item => item.Label.IndexOfCaseInsensitive(context.WordToComplete))
.ThenBy(static item => string.IsNullOrWhiteSpace(item.SortText) ? item.Label : item.SortText, StringComparer.OrdinalIgnoreCase);
Falls back to Label if SortText is null, empty, or whitespace, for the primary sort, and still hoists the matching values to the top first.
Also made the lambdas static since they may as well be.