fix(@angular/cli): improve error message for Windows autocompletion use cases #23075
Conversation
dgp1130
force-pushed
the
auto-completion-windows
branch
2 times, most recently
from
20d1520 to
37c667c
Compare
| export async function initializeAutocomplete(): Promise<string> { | ||
| // Windows doesn't support autocompletion, give a clear error message. | ||
| if (process.platform === 'win32') { | ||
| throw new Error(`Windows does not support shell autocompletion.`); |
There was a problem hiding this comment.
Auto complete should work on Windows as long as the user has bash or zsh installed, example through git bash, cmder etc..
The configs typically located in the user's profile directory
C:\Users\<username>\...
There was a problem hiding this comment.
I'm not sure about git bash but WSL 1/2 should have a platform of linux so that will be covered with that check.
There was a problem hiding this comment.
I am referring to usages outside of WSL.
On native Windows, zsh and bash can still be installed and available and autocompletion is still possible using;
Example using;
- cygwin
- cmder
- git bash
Considering a large set of our users are on Windows OS, I think we should support this case better. Alternatively, for the time being we can say that the automated setup doesn't support Windows but explain how this can be done by modifying the zsh and bash profiles manually.
There was a problem hiding this comment.
We could also check the OSTYPE environment variable when on Windows. I believe it should be msys or cygwin for those cases but would need to confirm.
There was a problem hiding this comment.
Yeah, I was going to check Git Bash today, as I suspect that will be win32. I forgot about Cygwin, but I could test that too. Cmder I thought was just a terminal which still used cmd under the hood and would be win32 (it's not a Linux emulator IIRC)? WSL should definitely be detected as Linux.
I'm wondering if a better approach might be to check $SHELL for cmd.exe or powershell.exe and print an error message on that case? Not sure if there are other Windows shells which might come up there.
There was a problem hiding this comment.
Just checked my personal machine and process.platform + $SHELL for each environment is:
- Cmd has platform
win32and$SHELLis unset - should fail with a useful error message. - Powershell has platform
win32and$SHELLis unset - should fail with a useful error message. - WSL2 has platform
linuxand shell/usr/bin/zsh- "just works". - Git Bash has platform
win32and shellC:\Program files\Git\usr\bin\bash.exe- NVM autocompletion works, so I suspect we should too, but I wasn't able to get it to use a local CLI build to confirm. - Cmder has platform
win32and$SHELLis unset, but I do see$CMDER_SHELL="cmd"- I suspect this shouldn't work and we should fail with a useful error message. - Cygwin couldn't install Node in a reasonable time frame, so I gave up there - I suspect this is similar to Git Bash and probably falls into that case.
All the unsupported systems have an unset $SHELL, so I think that is the best way of detecting the failure. Unfortunately there is no good way to disambiguate between the environments (Powershell doesn't seem to have any good environment variable to identify itself for instance). I'm inclined to fail with a more generic message on missing $SHELL along the lines of:
$SHELLenvironment variable not set. Only Bash and Zsh are supported. If you're on Windows, Cmd and Powershell don't support command autocompletion, but Git Bash or Windows Subsystem for Linux should work, so make sure to run the command from there.
(Sidebar: We should have a more directed/"goal-oriented" doc for setting up autocompletion which can explain the supported shells and link to relevant pieces.)
dgp1130
force-pushed
the
auto-completion-windows
branch
2 times, most recently
from
4857b17 to
9f201e5
Compare
dgp1130
changed the title
| export default async function () { | ||
| if (process.platform === 'win32') { | ||
| await windowsTests(); | ||
| return; // Windows should never prompt for autocompletion. |
There was a problem hiding this comment.
Would this fail if the test was run on Windows with git-bash?
CircleCI is setup to use powershell so CI would be fine. maybe expand the comment to note that.
There was a problem hiding this comment.
Yes, I think this would fail for Git Bash, I wasn't too concerned since CI isn't doing that.
It kinda sucks since any contributors using Git Bash will run the wrong set of tests and possibly get false negatives. I'm not too worried about it right now though, and I think we can figure that out when someone actually tries to do it, resolves all the other issues they'd run into with Git Bash in our setup (no idea how it plays with Bazel for instance), and can more easily find the right detection mechanism.
Updated the comment for now to be clear that only Cmd and Powershell shouldn't prompt.
…se cases Windows Cmd and Powershell don't support autocompletion, but it can be done with utilities like Windows Subsystem for Linux and Git Bash, which should "just work" due to emulating a Linux environment. This clarifies the error message most users will see to call out the state of the world with regard to autocompletion on Windows platforms.
dgp1130
force-pushed
the
auto-completion-windows
branch
from
9f201e5 to
77e29e4
Compare
dgp1130
added
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Autocompletion tests were failing on Windows due to a missing
$HOMEvariable, presumably Windows wasn't inheriting the$HOMEvariable like Linux does?Also fixing a semi-related typo I happened to notice around the same time.
Not sure exactly how to test this on Windows before merging, just hope for the best I guess?