Tyriar / vscode-terminal-here

VS Code extensions that creates an integrated terminal session at the current file's directory
https://marketplace.visualstudio.com/items?itemName=Tyriar.vscode-terminal-here
33 stars 11 forks source link

Incorrect detection of ubuntu subsystem #15

Closed furudean closed 5 years ago

furudean commented 6 years ago

When you run the command and your default terminal is the Ubuntu subsystem for Windows, the extension still applies the default behavior for Windows, giving an incorrect path:

cd "c:\Users\Whimzee\Desktop\project\"
$ cd "c:\Users\Ben\Desktop\project\"
-bash: cd: c:\Users\Whimzee\Desktop\project\: No such file or directory

This is how I have my terminal path set up in settings, which is default for the current distribution:

{
    "terminal.integrated.shell.windows": "ubuntu.exe"
}

My understanding is that something must be wrong with how the extension detects the Ubuntu subsystem.

furudean commented 6 years ago

I ended up solving this by instead using wsl.exe, which works flawlessly for my purposes (open integrated terminal in project folder by default) but the problem still persists when using terminalHere.create.

0xabu commented 6 years ago

I've just hit this as well. The extension appears to be assuming that a WSL shell can only be created by %windir%\System32\bash.exe, whereas in more recent WSL releases it might be ubuntu.exe, wsl.exe, or even some other wrapper. I think we'd be better off with a config flag to enable this behaviour rather than second-guessing it based on the shell path.

0xabu commented 6 years ago

BTW, on a modern WSL it's not safe to assume that C: is /mnt/c. The reliable thing to do is probably just invoke wslpath; e.g. you could emit a command like:

cd $(wslpath "${dir}")

Ref: https://docs.microsoft.com/en-us/windows/wsl/release-notes#build-17046 Unfortunately I don't see a good way to detect the existence of wslpath.

Tyriar commented 6 years ago

@0xabu

BTW, on a modern WSL it's not safe to assume that C: is /mnt/c. The reliable thing to do is probably just invoke wslpath

Is there a way to calculate the wslpath without shelling out? Extra IO on a command could push out the time it takes to run.

whereas in more recent WSL releases it might be ubuntu.exe, wsl.exe, or even some other wrapper.

Are ubuntu.exe and wsl.exe also in System32? And can you elaborate what you mean by some other wrapper? VS Code needs to be able to reliably detect this.

0xabu commented 6 years ago

I don't know of a way to do the path translation without invoking wslpath, sorry. Most users will have the default drive mappings under /mnt/X, but with automount/fstab support in WSL this is now all user-configurable.

If you want a wsl shell using the user's "default" distribution, then you can just invoke wsl.exe. But IIRC your extension is doing the reverse -- it's looking at the user's shell path and trying to decide "is this WSL?", and AFAIK there's no reliable way to do that. That's why I'd suggest having an explicit config flag the user has to set to enable WSL treatment. It's probably also safe to have a list of known WSL launchers (System32\bash.exe, System32\wsl.exe) and default the WSL flag on if one of those is seen.

Tyriar commented 6 years ago

I don't know of a way to do the path translation without invoking wslpath, sorry. Most users will have the default drive mappings under /mnt/X, but with automount/fstab support in WSL this is now all user-configurable.

I guess for link detection in VS Code will need to query wslpath once for each drive when links appear (currently links don't work at all).

f you want a wsl shell using the user's "default" distribution, then you can just invoke wsl.exe.

Is this reliable? I thought bash.exe was the only available option initially? Currently setting the terminal in VS Code to "WSL Bash" will use bash.exe: https://github.com/Microsoft/vscode/blob/6d5c53808d4f9967e446ec4f4b857ef394829850/src/vs/workbench/parts/terminal/electron-browser/terminalService.ts#L181

That's why I'd suggest having an explicit config flag the user has to set to enable WSL treatment. It's probably also safe to have a list of known WSL launchers (System32\bash.exe, System32\wsl.exe) and default the WSL flag on if one of those is seen.

We'll need as reliable method as possible to detect this as a whole bunch of things change behavior and we can't rely on users going into settings to configure this, they'll just see broken features if it's not working properly (eg. https://github.com/Microsoft/vscode/issues/36897). Also a global setting won't really give the ideal experience as the terminal is meant to be agile enough to jump between powershell/cmd/bash/etc. and still have everything work.

0xabu commented 6 years ago

See https://docs.microsoft.com/en-us/windows/wsl/wsl-config#set-a-default-distribution ... bash.exe and wsl.exe (both in System32) are safe indicators of a WSL shell. There's also the distro-specific command, which appears to be a stub exe at %LOCALAPPDATA%\Microsoft\WindowsApps\package where package is something like "CanonicalGroupLimited.UbuntuonWindows_79rhkp1fndgsc". I suspect you can ignore that last one as it's unlikely to be something the average user configures.

Maples7 commented 6 years ago

https://github.com/Maples7/vscode-terminal-inplace/issues/1

How about vscode.window.createTerminal({ cwd: <DirPath> })?

Tyriar commented 5 years ago

Fixed in https://github.com/Tyriar/vscode-terminal-here/pull/18