Skip to content

Make TabCompletion case-insensitive for file names on Unix #4699

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

Closed
iSazonov wants to merge 4 commits into from
Closed

Make TabCompletion case-insensitive for file names on Unix #4699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f772579
Make TabCompletion case-insensivite for file names on Unix
iSazonov Aug 29, 2017
aba45ac
Address feedbacks
iSazonov Aug 30, 2017
0f44e34
Fix test
iSazonov Aug 30, 2017
fb1eb1b
Add case-sensitive sorting for Unix
iSazonov Sep 8, 2017
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
68 changes: 62 additions & 6 deletions src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4061,10 +4061,8 @@ internal static IEnumerable<CompletionResult> CompleteFilename(CompletionContext
var relativePaths = context.GetOption("RelativePaths", @default: defaultRelative);
var useLiteralPath = context.GetOption("LiteralPaths", @default: false);

if (useLiteralPath && LocationGlobber.StringContainsGlobCharacters(wordToComplete))
{
wordToComplete = WildcardPattern.Escape(wordToComplete, Utils.Separators.StarOrQuestion);
}
// We should always escape '[' and ']' if present.
wordToComplete = WildcardPattern.Escape(wordToComplete, Utils.Separators.StarOrQuestion);

@lzybkr lzybkr Sep 5, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like the code was correct before - only when we know we have -LiteralPath.


if (!defaultRelative && wordToComplete.Length >= 2 && wordToComplete[1] == ':' && char.IsLetter(wordToComplete[0]) && executionContext != null)
{
Expand All @@ -4073,6 +4071,43 @@ internal static IEnumerable<CompletionResult> CompleteFilename(CompletionContext
executionContext.SessionState.Drive.GetAtScope(wordToComplete.Substring(0, 1), "global");
}

#if UNIX
// We use globbing to get completions.
// Globbing is based on system functions which is case-sensitive on Unix.
// To make globbing on Unix case-insensitive we transform every char from the input string:
// "alias:dir" -> "alias:[dD][iI][rR]"
if (context.GetOption("UnixCaseInsensitiveGlobbing", @default: true) == true)
{
StringBuilder sb = new StringBuilder(wordToComplete.Length * 4);

//wordToComplete = WildcardPattern.Escape(wordToComplete, Utils.Separators.Brackets);

@lzybkr lzybkr Aug 30, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove, but it might be worth a comment saying we assume wordToComplete is already a wildcard pattern and do not want to escape anything.

@iSazonov iSazonov Aug 31, 2017

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Removed.
Comment added above in line 4114.

Previous I removed there if useLiteralPath - Please clarify the if - it seems we should
(1) always escape '[' and ']'
(2) escape StarOrQuestion for useLiteralPath but we did not this previous and don't now.

@lzybkr lzybkr Sep 4, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The word to complete is usually a wildcard pattern (unless the filename specifies -LiteralPath, but I don't think we do that today).

So I think it's probably best to not escape anything. It is probably uncommon to use tab completion and square brackets together, but I'm sure I've done it before, so it might feel broken.

This does mean you need to detect letters inside square brackets and ignore those.

@iSazonov iSazonov Sep 5, 2017

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

So we leave as is now and add escape StarOrQuestion for useLiteralPath when we'll need?

@lzybkr lzybkr Sep 5, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, except I missed the code above that already does that, except you changed it.

In reply to: 136893930 [](ancestors = 136893930)

// Copy a provider name "as is" without transformation.
var providerPrefixIndex = wordToComplete.IndexOf(':') + 1;
int i;
for (i=0; i < providerPrefixIndex; i++)
{
sb.Append(wordToComplete[i]);
}

// Transform rest of input string after a provider prefix.
for (var j=i; j < wordToComplete.Length; j++)
{
var lch = Char.ToLower(wordToComplete[j]);
var uch = Char.ToUpper(wordToComplete[j]);
if (lch != uch)
{
sb.Append('[').Append(lch).Append(uch).Append(']');
}
else
{
sb.Append(wordToComplete[j]);
}
}

wordToComplete = sb.ToString();
}
#endif
var powerShellExecutionHelper = context.Helper;
powerShellExecutionHelper
.AddCommandWithPreferenceSetting("Microsoft.PowerShell.Management\\Resolve-Path")
Expand Down Expand Up @@ -4215,7 +4250,7 @@ internal static IEnumerable<CompletionResult> CompleteFilename(CompletionContext
}

// Sorting the results by the path
var sortedPsobjs = psobjs.OrderBy(a => a, new ItemPathComparer());
var sortedPsobjs = psobjs.OrderBy(a => a, new ItemPathComparer(wordToComplete));

foreach (PSObject psobj in sortedPsobjs)
{
Expand Down Expand Up @@ -6637,6 +6672,13 @@ internal static bool IsAmpersandNeeded(CompletionContext context, bool defaultCh

private class ItemPathComparer : IComparer<PSObject>
{
private String _baseWord;

public ItemPathComparer(String baseWord)
{
_baseWord = baseWord + ".*";
}

public int Compare(PSObject x, PSObject y)
{
var xPathInfo = PSObject.Base(x) as PathInfo;
Expand Down Expand Up @@ -6666,7 +6708,21 @@ public int Compare(PSObject x, PSObject y)
if (string.IsNullOrEmpty(xPath) || string.IsNullOrEmpty(yPath))
Diagnostics.Assert(false, "Base object of item PSObject should be either PathInfo or FileSystemInfo");

return String.Compare(xPath, yPath, StringComparison.CurrentCultureIgnoreCase);
var result = String.Compare(xPath, yPath, StringComparison.CurrentCultureIgnoreCase);
#if UNIX
if (result == 0)
{
if (Regex.IsMatch(xPath, _baseWord))
{
return -1;
}
else
{
return 1;
}
}
#endif
return result;
}
}

Expand Down
39 changes: 36 additions & 3 deletions test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,12 @@ Describe "TabCompletion" -Tags CI {

Context "File name completion" {
BeforeAll {
$oneSubDirLowerTest = "onesubd"
$oneSubDirUpperTest = "oneSubD"
$oneSubDirTest = "onesubdir"

$tempDir = Join-Path -Path $TestDrive -ChildPath "baseDir"
$oneSubDir = Join-Path -Path $tempDir -ChildPath "oneSubDir"
$oneSubDir = Join-Path -Path $tempDir -ChildPath $oneSubDirTest
$oneSubDirPrime = Join-Path -Path $tempDir -ChildPath "prime"
$twoSubDir = Join-Path -Path $oneSubDir -ChildPath "twoSubDir"

Expand All @@ -207,6 +211,13 @@ Describe "TabCompletion" -Tags CI {
New-Item -Path $oneSubDirPrime -ItemType Directory -Force > $null
New-Item -Path $twoSubDir -ItemType Directory -Force > $null

if (!$IsWindows) {
$oneSubDirTest2 = "oneSubDir"
$oneSubDir2 = Join-Path -Path $tempDir -ChildPath $oneSubDirTest2
New-Item -Path $oneSubDir2 -ItemType Directory -Force > $null
}


$testCases = @(
@{ inputStr = "ab"; name = "abc"; localExpected = ".${separator}abc"; oneSubExpected = "..${separator}abc"; twoSubExpected = "..${separator}..${separator}abc" }
@{ inputStr = "asaasas"; name = "asaasas!popee"; localExpected = ".${separator}asaasas!popee"; oneSubExpected = "..${separator}asaasas!popee"; twoSubExpected = "..${separator}..${separator}asaasas!popee" }
Expand Down Expand Up @@ -241,6 +252,28 @@ Describe "TabCompletion" -Tags CI {
Pop-Location
}

It "TabCompletion should be case-insensitive for file names on Windows" -Skip:(!$IsWindows) {
Push-Location -Path $tempDir
$res = TabExpansion2 -inputScript $oneSubDirLowerTest -cursorColumn $oneSubDirLowerTest.Length
$res.CompletionMatches.Count | Should BeGreaterThan 0
$res.CompletionMatches[0].CompletionText | Should Be ".${separator}$oneSubDirTest"

$res = TabExpansion2 -inputScript $oneSubDirUpperTest -cursorColumn $oneSubDirUpperTest.Length
$res.CompletionMatches.Count | Should BeGreaterThan 0
$res.CompletionMatches[0].CompletionText | Should Be ".${separator}$oneSubDirTest"
}

It "TabCompletion should be case-sensitive for file names on Unix" -Skip:($IsWindows) {
Push-Location -Path $tempDir
$res = TabExpansion2 -inputScript $oneSubDirLowerTest -cursorColumn $oneSubDirLowerTest.Length
$res.CompletionMatches.Count | Should BeGreaterThan 0
$res.CompletionMatches[0].CompletionText | Should BeExactly ".${separator}$oneSubDirTest"

$res = TabExpansion2 -inputScript $oneSubDirUpperTest -cursorColumn $oneSubDirUpperTest.Length
$res.CompletionMatches.Count | Should BeGreaterThan 0
$res.CompletionMatches[0].CompletionText | Should BeExactly ".${separator}$oneSubDirTest2"
}

It "Input '<inputStr>' should successfully complete" -TestCases $testCases {
param ($inputStr, $localExpected)

Expand Down Expand Up @@ -478,8 +511,8 @@ Describe "TabCompletion" -Tags CI {
$beforeTab = 'filesystem::{0}\Wind' -f $env:SystemDrive
$afterTab = 'filesystem::{0}\Windows' -f $env:SystemDrive
} else {
$beforeTab = 'filesystem::/us' -f $env:SystemDrive
$afterTab = 'filesystem::/usr' -f $env:SystemDrive
$beforeTab = 'filesystem::/sbi' -f $env:SystemDrive
$afterTab = 'filesystem::/sbin' -f $env:SystemDrive
}
$res = TabExpansion2 -inputScript $beforeTab -cursorColumn $beforeTab.Length
$res.CompletionMatches.Count | Should BeGreaterThan 0
Expand Down