terminal: Windows .exe compatibility for VSCode #12761
Conversation
|
Yeah, I can see why the test failed and what I did wrong. Easy to fix. |
|
Commit e0e2b2e fixes the problem that broke the test and adds a new test case for this PR's Windows-specific behaviour. |
There was a problem hiding this comment.
LGTM. I've tested with the instructions from the ticket.
Besides the refactoring with the introduction of the createPseudoTerminal method, the only code change is the addition of the if (isWindows && command && !command.toLowerCase().endsWith('.exe')) clause, which looks good to me.
| if (message.startsWith('File not found: ') || message.endsWith(NodePtyErrors.ENOENT)) { | ||
| if (isWindows && command && !command.toLowerCase().endsWith('.exe')) { | ||
| const commandExe = command + '.exe'; | ||
| this.logger.warn(`Trying terminal command '${commandExe}' because '${command}' was not found.`); |
There was a problem hiding this comment.
I don't have a strong opinion here, but imo this could be an info message as well.
I guess it depends on whether we see this more as a feature or more as a fallback for a misconfiguration
There was a problem hiding this comment.
Actually, that's a good point. I might rather lean to 'debug' as this is supposed to be unsurprising and totally supported behaviour, but should still be recorded in case some flow doesn't happen as expect. For example, in the — decidedly inadvisable — case where a .exe file exists but the developer doesn't want it to be used in the case that the unsuffixed file doesn't exist?
There was a problem hiding this comment.
Changed in commit dcdd3f1 to debug level.
cdamus
force-pushed
the
issue/12734
branch
2 times, most recently
from
b5d02c6
to
348dd1e
Compare
|
I should mention also that this PR fixes deprecation warnings in the listener API usage of the |
Support implicit '.exe' extension on the shell command in terminal profiles as in VS Code. Fixes eclipse-theia#12734 Signed-off-by: Christian W. Damus <cdamus.ext@eclipsesource.com>
vince-fugnitto
added
terminal
What it does
On Windows platform, support implicit file '.exe' extension on the shell command in terminal profiles as in VS Code.
VS Code allows a terminal profile to omit the
'.exe'extension of the executable file and will find and run it anyways. So this PR enhances theTerminalProcessinitialization, in the case that the program launch fails by reason of not being found and the host platform is Windows and the command did not end with'.exe', to try again with'.exe'appended to the program.Fixes #12734
How to test
Per the description of #12734:
terminalbranch. This provides amyextVS Code extension that you can link into yourplugins/directory to obtain a "Profile from my extension" terminal profile. This is configured simply with'cmd'as the command to run, not'cmd.exe'.Review checklist
Reminder for reviewers