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.

Sign up for GitHub

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

Jump to bottom

Use single best completion item for commands, options, and global options #1700

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use single best completion item for commands, options, and global options #1700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/System.CommandLine.Suggest.Tests/DotnetSuggestEndToEndTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void Dotnet_suggest_provides_suggestions_for_app_with_only_commandname()

stdOut.ToString()
.Should()
.Be($"--apple{NewLine}--banana{NewLine}--cherry{NewLine}--durian{NewLine}--help{NewLine}--version{NewLine}-?{NewLine}-h{NewLine}/?{NewLine}/h{NewLine}");
.Be($"--apple{NewLine}--banana{NewLine}--cherry{NewLine}--durian{NewLine}--help{NewLine}--version{NewLine}");
}
}
}
38 changes: 26 additions & 12 deletions src/System.CommandLine/Command.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,13 @@ public override IEnumerable<CompletionItem> GetCompletions(CompletionContext con
var commands = Subcommands;
for (int i = 0; i < commands.Count; i++)
{
AddCompletionsFor(commands[i]);
AddCompletionFor(commands[i], textToMatch);
}

var options = Options;
for (int i = 0; i < options.Count; i++)
{
AddCompletionsFor(options[i]);
AddCompletionFor(options[i], textToMatch);
}

var arguments = Arguments;
Expand All @@ -214,7 +214,7 @@ public override IEnumerable<CompletionItem> GetCompletions(CompletionContext con

if (option.IsGlobal)
{
AddCompletionsFor(option);
AddCompletionFor(option, textToMatch);
}
}
}
Expand All @@ -225,17 +225,31 @@ public override IEnumerable<CompletionItem> GetCompletions(CompletionContext con
.OrderBy(item => item.SortText.IndexOfCaseInsensitive(context.WordToComplete))
.ThenBy(symbol => symbol.Label, StringComparer.OrdinalIgnoreCase);

void AddCompletionsFor(IdentifierSymbol identifier)
// 'best' is a bit of a misnomer here. We want to return one and only one completion itme for each option,
// but depending on what has already been entered by the user the algorithm changes.
// For empty input, we return the longest alias of the set of aliases (this matches the DefaultName logic in Option.cs, but does not remove
// any prefixes (--, etc)).
// For nonempty input, we find all tokens that contain the input and then return the first one sorted by:
// * shortest Levenstein distance, then by
// * longest common startswith substring
string? FindBestCompletionFor(string textToMatch, IReadOnlyCollection<string> aliases) => textToMatch switch
{
#if NET6_0_OR_GREATER
"" => aliases.MaxBy(a => a.Length), // find the longest alias
(string stem) => aliases.Where(a => a.Contains(stem, StringComparison.OrdinalIgnoreCase)).OrderByDescending(a => TokenDistances.GetLevensteinDistance(stem, a)).ThenByDescending(a => TokenDistances.GetStartsWithDistance(stem, a)).FirstOrDefault()
Copy link
Member Author

Choose a reason for hiding this comment

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

use alias.ContainsCaseInsensitive extension method instead here to remove TFM split

#else
"" => aliases.OrderByDescending(a => a.Length).FirstOrDefault(), // find the longest alias
(string stem) => aliases.Where(a => a.Contains(stem)).OrderByDescending(a => TokenDistances.GetLevensteinDistance(stem, a)).ThenByDescending(a => TokenDistances.GetStartsWithDistance(stem, a)).FirstOrDefault()
#endif
};

void AddCompletionFor(IdentifierSymbol identifier, string textToMatch)
{
if (!identifier.IsHidden)
{
foreach (var alias in identifier.Aliases)
{
if (alias is { } &&
alias.ContainsCaseInsensitive(textToMatch))
{
completions.Add(new CompletionItem(alias, CompletionItemKind.Keyword, detail: identifier.Description));
}
{
var bestAlias = FindBestCompletionFor(textToMatch, identifier.Aliases);
if (bestAlias is not null) {
completions.Add(new CompletionItem(bestAlias, CompletionItemKind.Keyword, detail: identifier.Description));
}
}
}
Expand Down
85 changes: 4 additions & 81 deletions src/System.CommandLine/Invocation/TypoCorrection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,17 @@ private IEnumerable<string> GetPossibleTokens(Command targetSymbol, string token
.Select(symbol =>
symbol.Aliases
.Union(symbol.Aliases)
.OrderBy(x => GetDistance(token, x))
.ThenByDescending(x => GetStartsWithDistance(token, x))
.OrderBy(x => TokenDistances.GetLevensteinDistance(token, x))
.ThenByDescending(x => TokenDistances.GetStartsWithDistance(token, x))
.First()
);

int? bestDistance = null;
return possibleMatches
.Select(possibleMatch => (possibleMatch, distance:GetDistance(token, possibleMatch)))
.Select(possibleMatch => (possibleMatch, distance: TokenDistances.GetLevensteinDistance(token, possibleMatch)))
.Where(tuple => tuple.distance <= _maxLevenshteinDistance)
.OrderBy(tuple => tuple.distance)
.ThenByDescending(tuple => GetStartsWithDistance(token, tuple.possibleMatch))
.ThenByDescending(tuple => TokenDistances.GetStartsWithDistance(token, tuple.possibleMatch))
.TakeWhile(tuple =>
{
var (_, distance) = tuple;
Expand All @@ -71,82 +71,5 @@ private IEnumerable<string> GetPossibleTokens(Command targetSymbol, string token
})
.Select(tuple => tuple.possibleMatch);
}

private static int GetStartsWithDistance(string first, string second)
{
int i;
for (i = 0; i < first.Length && i < second.Length && first[i] == second[i]; i++)
{ }
return i;
}

//Based on https://blogs.msdn.microsoft.com/toub/2006/05/05/generic-levenshtein-edit-distance-with-c/
private static int GetDistance(string first, string second)
{
// Validate parameters
if (first is null)
{
throw new ArgumentNullException(nameof(first));
}

if (second is null)
{
throw new ArgumentNullException(nameof(second));
}


// Get the length of both. If either is 0, return
// the length of the other, since that number of insertions
// would be required.

int n = first.Length, m = second.Length;
if (n == 0) return m;
if (m == 0) return n;


// Rather than maintain an entire matrix (which would require O(n*m) space),
// just store the current row and the next row, each of which has a length m+1,
// so just O(m) space. Initialize the current row.

int curRow = 0, nextRow = 1;
int[][] rows = { new int[m + 1], new int[m + 1] };

for (int j = 0; j <= m; ++j)
{
rows[curRow][j] = j;
}

// For each virtual row (since we only have physical storage for two)
for (int i = 1; i <= n; ++i)
{
// Fill in the values in the row
rows[nextRow][0] = i;
for (int j = 1; j <= m; ++j)
{
int dist1 = rows[curRow][j] + 1;
int dist2 = rows[nextRow][j - 1] + 1;
int dist3 = rows[curRow][j - 1] + (first[i - 1].Equals(second[j - 1]) ? 0 : 1);

rows[nextRow][j] = Math.Min(dist1, Math.Min(dist2, dist3));
}


// Swap the current and next rows
if (curRow == 0)
{
curRow = 1;
nextRow = 0;
}
else
{
curRow = 0;
nextRow = 1;
}
}

// Return the computed edit distance
return rows[curRow][m];

}
}
}
86 changes: 86 additions & 0 deletions src/System.CommandLine/Utilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
using System.CommandLine.Parsing;
using System.IO;

namespace System.CommandLine;

internal static class TokenDistances
{

//Based on https://blogs.msdn.microsoft.com/toub/2006/05/05/generic-levenshtein-edit-distance-with-c/
public static int GetLevensteinDistance(string first, string second)
{
// Validate parameters
if (first is null)
{
throw new ArgumentNullException(nameof(first));
}

if (second is null)
{
throw new ArgumentNullException(nameof(second));
}


// Get the length of both. If either is 0, return
// the length of the other, since that number of insertions
// would be required.

int n = first.Length, m = second.Length;
if (n == 0) return m;
if (m == 0) return n;


// Rather than maintain an entire matrix (which would require O(n*m) space),
// just store the current row and the next row, each of which has a length m+1,
// so just O(m) space. Initialize the current row.

int curRow = 0, nextRow = 1;
int[][] rows = { new int[m + 1], new int[m + 1] };

for (int j = 0; j <= m; ++j)
{
rows[curRow][j] = j;
}

// For each virtual row (since we only have physical storage for two)
for (int i = 1; i <= n; ++i)
{
// Fill in the values in the row
rows[nextRow][0] = i;
for (int j = 1; j <= m; ++j)
{
int dist1 = rows[curRow][j] + 1;
int dist2 = rows[nextRow][j - 1] + 1;
int dist3 = rows[curRow][j - 1] + (first[i - 1].Equals(second[j - 1]) ? 0 : 1);

rows[nextRow][j] = Math.Min(dist1, Math.Min(dist2, dist3));
}


// Swap the current and next rows
if (curRow == 0)
{
curRow = 1;
nextRow = 0;
}
else
{
curRow = 0;
nextRow = 1;
}
}

// Return the computed edit distance
return rows[curRow][m];

}

///<summary>Measures the length of the common starting substring of two strings</summary>
public static int GetStartsWithDistance(string first, string second)
{
int i;
for (i = 0; i < first.Length && i < second.Length && first[i] == second[i]; i++)
{ }
return i;
}
}