Make TabCompletion case-insensitive for file names on Unix (2-nd attempt) by iSazonov · Pull Request #7221 · PowerShell/PowerShell

Ilya (iSazonov) · 2018-07-02T12:50:57Z

PR Summary

Attempt to fix #1273.

PR Checklist

…mpt)

Ilya (iSazonov) · 2018-07-30T09:40:35Z

Steve Lee (@SteveL-MSFT) sergei (@vors) Can we review and merge the PR before 6.1?

Steve Lee (SteveL-MSFT) · 2018-07-30T21:31:16Z

+            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"


I would expect the result casing to match the actual casing. Use -BeExactly

Steve Lee (SteveL-MSFT) · 2018-07-30T21:31:29Z

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


Shouldn't we know exactly the number of expected completions?

For this fix, it does not matter count (the check is only protection from exception in next line) - the order is important.

Since this test created the folder structure, the number of completions should be the same each time, right?

Yes. Count check was added.

Steve Lee (SteveL-MSFT) · 2018-07-30T21:31:43Z

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


Steve Lee (SteveL-MSFT) · 2018-07-30T21:31:56Z

+        It "TabCompletion should be case-sensitive for file names on Unix" -Skip:(!$IsLinux) {
+            Push-Location -Path $tempDir
+            $res = TabExpansion2 -inputScript $oneSubDirLowerTest -cursorColumn $oneSubDirLowerTest.Length
+            $res.CompletionMatches.Count | Should BeGreaterThan 0


Shouldn't we know exactly the number of expected completions?

Steve Lee (SteveL-MSFT) · 2018-07-30T21:32:03Z

+            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"


Steve Lee (SteveL-MSFT) · 2018-07-30T21:32:54Z

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


Shouldn't we know exactly how many results we should get since we control the folder structure?

Steve Lee (SteveL-MSFT) · 2018-07-30T21:32:24Z

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


Steve Lee (SteveL-MSFT) · 2018-07-30T21:33:41Z

+        It "TabCompletion should be case-insensitive for file names on Windows and MacOS" -Skip:($IsLinux) {
+            Push-Location -Path $tempDir
+            $res = TabExpansion2 -inputScript $oneSubDirLowerTest -cursorColumn $oneSubDirLowerTest.Length
+            $res.CompletionMatches.Count | Should BeGreaterThan 0


Shouldn't we know exactly the number of expected completions?

Steve Lee (SteveL-MSFT) · 2018-07-31T15:57:11Z

+        It "TabCompletion should be case-sensitive for file names on Unix" -Skip:(!$IsLinux) {
+            Push-Location -Path $tempDir
+            $res = TabExpansion2 -inputScript $oneSubDirLowerTest -cursorColumn $oneSubDirLowerTest.Length
+            $res.CompletionMatches | Should -HaveCount 2


This should still be -HaveCount 1? You're testing that "onesubd" and "oneSubD" both return "onesubdir", right?

It seems we should have a test for Linux for a directory structure like:

testfolder testFolder

and tabcomplete off test should return two results

This additional test case is still needed.

Steve Lee (@SteveL-MSFT) It is already there - see line 226. So I removed last commit. Seems my solution is not full - the count should be 2 on .
How can I get the artifact (compiled PowerShell) from CI Travis?

Ilya (@iSazonov) you're right, I misunderstood that test. Did a little investigation and it appears that the change to GetFileSystemEntries() is not sufficient as a call to Resolve-Path earlier only returns case-sensitive matches on Linux.

Steve Lee (@SteveL-MSFT) Should we open new issue to discuss making Resolve-Path case-insensitive? This is most likely to affect globbing whole.

Ilya (@iSazonov) we should probably have a new issue for Resolve-Path. May need to add a -CaseInsensitive switch to maintain backwards compatibility.

stale · 2018-09-01T06:23:56Z

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

Ilya (iSazonov) · 2018-09-03T05:37:11Z

I considered Resolve-Path -CaseInsensitive . Adding the parameter in APIs looks as hude change in engine (in public APIs too). Another way is to introduce new internal flag/property. I like this more but don't still get full understanding of the consequences (side affects). We could use the property to be case-insensitive in some scenarios on Unix-s.

Michael Klement (@mklement0) Maybe you have any thoughts about this?

stale · 2018-10-03T06:05:23Z

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

stale · 2018-10-13T06:18:30Z

This PR has be automatically close because it is stale. If you wish to continue working on the PR, please first update the PR, then reopen it.
Thanks again for your contribution.
Community members are welcome to grab these works.

Ilya (iSazonov) · 2018-10-15T09:26:46Z

The fix needs changes in some API levels. It is a lot of work...

Ilya (iSazonov) requested review from Bruce Payette (BrucePay) and Dongbo Wang (daxian-dbw) as code owners July 2, 2018 12:50

Make TabCompletion case-insensitive for file names on Unix (2-nd atte…

313755d

…mpt)

Ilya (iSazonov) force-pushed the unix-globbing-case-insensitive-2 branch from adaddb9 to 313755d Compare July 2, 2018 13:21

Fix tests on MacOs

be262de

Ilya (iSazonov) force-pushed the unix-globbing-case-insensitive-2 branch from bf9a4d5 to be262de Compare July 2, 2018 14:50

Ilya (iSazonov) requested a review from sergei (vors) July 2, 2018 15:13

Steve Lee (SteveL-MSFT) requested changes Jul 30, 2018

View reviewed changes

Ilya (iSazonov) added 2 commits July 31, 2018 08:45

Fix tests

54dd6f9

Add count check

8e3f64e

Steve Lee (SteveL-MSFT) requested changes Jul 31, 2018

View reviewed changes

Ilya (iSazonov) force-pushed the unix-globbing-case-insensitive-2 branch from e1c394b to 8e3f64e Compare August 1, 2018 04:44

stale Bot added the Stale label Sep 1, 2018

sergei (vors) removed their request for review September 1, 2018 11:17

stale Bot removed the Stale label Sep 1, 2018

Ilya (iSazonov) mentioned this pull request Sep 4, 2018

Set-Location preserves case instead of matching filesystem on case-insensitive/preserving system #5678

Closed

stale Bot added the Stale label Oct 3, 2018

stale Bot closed this Oct 13, 2018

Ilya (iSazonov) deleted the unix-globbing-case-insensitive-2 branch May 15, 2020 13:09

Conversation

Ilya (iSazonov) commented Jul 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Ilya (iSazonov) commented Jul 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale Bot commented Sep 1, 2018

Uh oh!

Ilya (iSazonov) commented Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stale Bot commented Oct 3, 2018

Uh oh!

stale Bot commented Oct 13, 2018

Uh oh!

Ilya (iSazonov) commented Oct 15, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Ilya (iSazonov) commented Jul 2, 2018 •

edited

Loading

Ilya (iSazonov) Aug 2, 2018 •

edited

Loading

Ilya (iSazonov) commented Sep 3, 2018 •

edited

Loading