Skip to content

fix(@angular/cli): skip prompt or warn when setting up autocompletion without a global CLI install #23145

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

Merged
merged 1 commit into from
May 18, 2022

Conversation

dgp1130
Copy link
Collaborator

@dgp1130 dgp1130 commented May 17, 2022

If the user does not have a global install of the Angular CLI, the autocompletion prompt is skipped and ng completion emits a warning. The reasoning for this is that source <(ng completion script) won't work without ng on the $PATH, which is only really viable with a global install. Local executions like git clone ... && npm install && npm start or ephemeral executions like npx @angular/cli don't benefit from autocompletion and unnecessarily impede users.

A global install of the Angular CLI is detected by running which -a ng, which appears to be a cross-platform means of listing all ng commands on the $PATH. We then look over all binaries in the list and exclude anything which is a directo child of a node_modules/.bin/ directory. These include local executions and npx, so the only remaining locations should be global installs (/usr/bin/ng, NVM, etc.).

The tests are a little awkward since ng is installed globally by helper functions before tests start. These tests uninstall the global CLI and install a local, project-specific version to verify behavior, before restoring the global version. Hypothetically this could be emulated by manipulating the $PATH variable, but which needs to be available (so we can't clobber the whole $PATH) and node exists in the same directory as the global ng command (so we can't remove that directory anyways). There's also no good way of testing the case where which fails to run.

Closes #23135.

/cc @alan-agius4

@dgp1130 dgp1130 added the target: rc This PR is targeted for the next release-candidate label May 17, 2022
@dgp1130 dgp1130 added this to the v14 milestone May 17, 2022
@dgp1130 dgp1130 requested a review from clydin May 17, 2022 00:35
@dgp1130 dgp1130 force-pushed the autocompletion-path branch from 3664b1e to ede4315 Compare May 17, 2022 00:37
@dgp1130 dgp1130 force-pushed the autocompletion-path branch from ede4315 to 5203a38 Compare May 17, 2022 00:57
@dgp1130 dgp1130 force-pushed the autocompletion-path branch 2 times, most recently from f34715f to cdb0fb2 Compare May 18, 2022 04:36
… without a global CLI install

If the user does not have a global install of the Angular CLI, the autocompletion prompt is skipped and `ng completion` emits a warning. The reasoning for this is that `source <(ng completion script)` won't work without `ng` on the `$PATH`, which is only really viable with a global install. Local executions like `git clone ... && npm install && npm start` or ephemeral executions like `npx @angular/cli` don't benefit from autocompletion and unnecessarily impede users.

A global install of the Angular CLI is detected by running `which -a ng`, which appears to be a cross-platform means of listing all `ng` commands on the `$PATH`. We then look over all binaries in the list and exclude anything which is a directo child of a `node_modules/.bin/` directory. These include local executions and `npx`, so the only remaining locations should be global installs (`/usr/bin/ng`, NVM, etc.).

The tests are a little awkward since `ng` is installed globally by helper functions before tests start. These tests uninstall the global CLI and install a local, project-specific version to verify behavior, before restoring the global version. Hypothetically this could be emulated by manipulating the `$PATH` variable, but `which` needs to be available (so we can't clobber the whole `$PATH`) and `node` exists in the same directory as the global `ng` command (so we can't remove that directory anyways). There's also no good way of testing the case where `which` fails to run.

Closes angular#23135.
@dgp1130 dgp1130 force-pushed the autocompletion-path branch from cdb0fb2 to 33ca5c9 Compare May 18, 2022 05:36
@dgp1130 dgp1130 removed the request for review from clydin May 18, 2022 17:25
@dgp1130 dgp1130 added the action: merge The PR is ready for merge by the caretaker label May 18, 2022
@dgp1130 dgp1130 merged commit b79b0f0 into angular:main May 18, 2022
@dgp1130 dgp1130 deleted the autocompletion-path branch May 18, 2022 17:34
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems installing ng14 cli completion on macOS
2 participants