Skip to content

$psEditor.Workspace.OpenFile puts file name into all lower case. #2960

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.

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
jhoneill opened this issue Sep 17, 2020 · 10 comments · Fixed by #4703
Closed

$psEditor.Workspace.OpenFile puts file name into all lower case. #2960

jhoneill opened this issue Sep 17, 2020 · 10 comments · Fixed by #4703
Assignees
Labels

Comments

@jhoneill
Copy link

jhoneill commented Sep 17, 2020

System Details Output


### VSCode version: 1.49.0 e790b931385d72cf5669fcefc51cdf65990efa5d x64

### VSCode extensions:      
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]
[email protected]


### PSES version: 2.3.0.0

### PowerShell version:

Name                           Value
----                           -----
PSVersion                      7.0.3
PSEdition                      Core
GitCommitId                    7.0.3
OS                             Microsoft Windows 10.0.18363
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

This also appears with vs-code insiders and the 2020.9.0 extension ...

The issue is that the $psEditor.Workspace.OpenFile - which is called from various places - like PsEdit, changes the file name to lower case. This is usually just a cosmetic problem, but I have found a case were pester ran a script using its lower-cased name and the script failed because it tried to do a case sensitive replace on part of its own name.

@ghost ghost added the Needs: Triage Maintainer attention needed! label Sep 17, 2020
@rjmholt
Copy link
Contributor

rjmholt commented Sep 17, 2020

@jhoneill can you give a particular example of where you've used this and what happened? There's not quite enough information here for me to work out what's going on and where the problem lies

@SeeminglyScience
Copy link
Collaborator

@rjmholt I think we store ScriptFile's as all lower case in an OrderedDictionary on Mac/Windows. I've ran into it too, easiest way to repro is:

# In PSIC 
New-Item Something.ps1 | Out-Null
psedit ./Something.ps1

And then look at the tab title.

I think it only happens when you haven't already opened the file at least once (so it's not in the workspace cache). I'm guessing the path is always lowercase in the request and if VSCode hasn't already cached some state for that file it'll take our word for it.

@rjmholt
Copy link
Contributor

rjmholt commented Sep 17, 2020

Sounds like our approach with surfacing the keys is wrong and they should be considered internal keys only, rather than something we expose somewhere.

Still, if I can get the example where it's caused a Pester issue in full detail, we'll be able to fix this in a much more reliable way than if I just guess where the issue is and claim it's solved.

@SydneyhSmith SydneyhSmith removed the Needs: Triage Maintainer attention needed! label Sep 17, 2020
@jhoneill
Copy link
Author

Still, if I can get the example where it's caused a Pester issue in full detail, we'll be able to fix this in a much more reliable way than if I just guess where the issue is and claim it's solved.

It's working with the module from https://github.com/MethodsAndPractices/vsteam and the author has
a) A PS1 file for each command for each command function (e.g. Add-VSTeam.ps1)
b) A .Tests.PS1 file for each (e.g. Add-VSTeam.Tests.ps1) as the unit test for that command.
c) Lines in every unit-test to dot-source all the files needed for that command.

Describe "VSTeam" {
  BeforeAll {  
    $sut = (Split-Path -Leaf $PSCommandPath).Replace(".Tests.", ".")
    . "$PSScriptRoot/../../Source/Public/$sut"

So I

  1. Open Add-VSTeam.Tests.ps1 from the PowerShell Window. It opens as add-vsteam.tests.ps1
    2.) I click Debug tests in the VSCode editor.

Pester then runs add-vsteam.tests.ps1 and $PSCommandPath).Replace(".Tests.", ".") does not replace tests, because this replace() is case sensitive. The script tires to DOT source a file with .tests and everything fails.

I know I could fix this by using -replace (or making other changes) but this bit of design isn't under my control. I can work round it by not opening from the command line ... and everywhere else - on windows at least - it is just a cosmetic thing. I don't know what happens if the underlying OS has a case sensitive file system.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Sep 18, 2020
@SydneyhSmith
Copy link
Collaborator

Thanks for the additional info @jhoneill

@SydneyhSmith SydneyhSmith added Area-psEditor Issue-Bug A bug to squash. and removed Needs: Maintainer Attention Maintainer attention needed! labels Sep 22, 2020
@ghost
Copy link

ghost commented Aug 19, 2021

This issue was closed automatically as repro info was indicated as needed, but there has been no activity in over a week. Please feel free to reopen with any available information!

@ghost ghost closed this as completed Aug 19, 2021
@jhoneill
Copy link
Author

jhoneill commented Aug 19, 2021

The bot auto closed this but the problem persists. I thought there was sufficient info to repro it ? Despite what the message, says it looks like one cannot reopen an issue which has has been closed by the bot.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Aug 19, 2021
@andyleejordan
Copy link
Member

Oops, we just updated the bot rules. Looks like it was closed because it was still marked as needing repro info. I'll reopen and try to repro with the info you've given.

@andyleejordan
Copy link
Member

@rjmholt I think we store ScriptFile's as all lower case in an OrderedDictionary on Mac/Windows. I've ran into it too, easiest way to repro is:

I saw this when bringing the debug service unit tests back online...it's strange.

@andyleejordan andyleejordan removed the Needs: Maintainer Attention Maintainer attention needed! label Aug 24, 2021
@fflaten
Copy link
Contributor

fflaten commented Feb 20, 2023

private async openFile(openFileDetails: IOpenFileDetails): Promise<EditorOperationResponse> {
const filePath = await this.normalizeFilePath(openFileDetails.filePath);
const doc = await vscode.workspace.openTextDocument(filePath);

normalizeFilePath() lowercases the path on Windows and MacOS. Maybe path validation (for casing errors etc) could be done server-side? Ex. Open-EditorFile already validates the path, but using $psEditor API directly doesn't.

@andyleejordan andyleejordan added this to the Consider-vNext milestone Feb 23, 2023
@SydneyhSmith SydneyhSmith moved this to Todo in Flying Fox Mar 15, 2023
andyleejordan added a commit that referenced this issue Aug 16, 2023
Per https://nodejs.org/en/docs/guides/working-with-different-filesystems
we should really just keep the file path we're given, no "normalizing"
to be done especially since it is wrong to infer from the OS if the
filesystem is case-sensitive or not. Gone are the days where Windows and
macOS could be presumed case-insensitive, they now both support and are
often found with with case-sensitive filesystems. Plus, there was really
no reason to be doing this in the first place. Tested interactively,
this works just fine since the VS Code APIs we're using handle
case-insensitivity for us.

Resolves #2960.
@andyleejordan andyleejordan moved this from Todo to In Progress in Flying Fox Aug 16, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 17, 2023
Per https://nodejs.org/en/docs/guides/working-with-different-filesystems
we should really just keep the file path we're given, no "normalizing"
to be done especially since it is wrong to infer from the OS if the
filesystem is case-sensitive or not. Gone are the days where Windows and
macOS could be presumed case-insensitive, they now both support and are
often found with with case-sensitive filesystems. Plus, there was really
no reason to be doing this in the first place. Tested interactively,
this works just fine since the VS Code APIs we're using handle
case-insensitivity for us.

Resolves #2960.
@github-project-automation github-project-automation bot moved this from In Progress to Done in Flying Fox Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants