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.

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

Windows drive letter case different between ${workspaceFolder} and integratedTerminal #42159

Closed
farrago opened this issue Jan 25, 2018 · 6 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug config VS Code configuration, set up issues verified Verification succeeded
Milestone

Comments

@farrago
Copy link

farrago commented Jan 25, 2018

  • VSCode Version: Code 1.19.1 (0759f77, 2017-12-19T09:46:23.884Z)
  • OS Version: Windows_NT x64 10.0.16299
  • Extensions: none

Steps to Reproduce:

  1. Create trivial js file to log __dirname and the first argument to your program. e.g.
    console.log('__dirname', __dirname);
    console.log('${workspaceFolder}', process.argv[2]);
  2. "File" -> "Open Folder", then navigate to and select the directory with the above file in it.
  3. Create a default node debug config in launch.json
  4. Add "args": ["${workspaceFolder}"] to pass it as the first param (which will be logged above
  5. Add "console": "integratedTerminal" to use integrated terminal. This gives a launch.json of:
        {
        // Use IntelliSense to learn about possible attributes.
        // Hover to view descriptions of existing attributes.
        // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
        "version": "0.2.0",
        "configurations": [
            {
                "type": "node",
                "request": "launch",
                "name": "Launch Program",
                "program": "${workspaceFolder}/test.js",
                "args": [
                    "${workspaceFolder}"
                ],
                "console": "integratedTerminal"
            }
        ]
    }

Actual Output:

D:\test>cd d:\test && node --inspect-brk=15398 test.js d:\test
Debugger listening on ws://127.0.0.1:15398/5d5674d0-9a73-4e7a-bb58-658d7ca0d24f
For help see https://nodejs.org/en/docs/inspector
Debugger attached.
__dirname D:\test
${workspaceFolder} d:\test
Waiting for the debugger to disconnect...

D:\test>

Notes:

  1. The path in the terminal window uses a capital "D".
  2. The internal path appears to use a lower case "d". This can be seen in both the logged value of ${workspaceFolder}, and in the command line that launches the debug session.
  3. If using, "Command Prompt" console, it is possible to manually change the current directory to a lowercase drive letter to make them match, but that is not the default.

Expected Output:

The drive letter case used by the integrated terminal and that used by the integrated terminal should match.

This is important because node considers files with a different case to be different files during require(). Passing paths to a node script that are built using ${workspaceFolder} and using them to require files causes different casing between otherwise identical filepaths.

Given that:

  1. The file explorer with which the folder was selected in "File" -> "Open Folder" showed upper case "D"
  2. AFAIK, upper case is the canonical form in Windows (as enforced in Explorer, Powershell, etc.)
  3. I believe it is impossible to make Powershell (the default integratedTerminal) use lower case drive letters.

Then VSCode should internally use the uppercase form.


Notes

@isidorn
Copy link
Contributor

isidorn commented Jan 25, 2018

@bpasero duplicate of #12448 ?
Since the workspace folder path should not enter lowercase in our sistem in the first place. When I am replacing in the configuration resovler service I simply take what the context service gives me

@bpasero
Copy link
Member

bpasero commented Jan 25, 2018

@isidorn do you mean #12448 ?

@bpasero
Copy link
Member

bpasero commented Jan 25, 2018

@isidorn as far as I know we do some normalization of drive letters in the URI class (@jrieken can correct me if I am wrong), so I would assume that at least as long as URIs are being used, we are consistent everywhere.

It is true however that we have a helper method normalizeDriveLetter that we use whenever a URI path is being displayed in the UI because indeed on Windows the default seems to be uppercase.

Ideally fsPath would return uppercase drive letters on Windows imho.

@isidorn
Copy link
Contributor

isidorn commented Jan 26, 2018

@bpasero yes I meant that issue number (just edited my mistake)

@jrieken
Copy link
Member

jrieken commented Jan 26, 2018

I know we do some normalization of drive letters in the URI class (@jrieken can correct me if I am wrong)

We lower-case it for some historic reason... I am not brave enough to change that TBH

@bpasero
Copy link
Member

bpasero commented Jan 26, 2018

Yeah it could break quite a bit of things. @isidorn maybe at your point you should just normalize the drive letter then? It does not seem to be an issue in any other places so far except this one.

@isidorn isidorn added bug Issue identified by VS Code Team member as probable bug config VS Code configuration, set up issues labels Jan 29, 2018
@isidorn isidorn added this to the January 2018 milestone Jan 29, 2018
@ramya-rao-a ramya-rao-a added the verified Verification succeeded label Feb 2, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug config VS Code configuration, set up issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants