feat(@angular/cli): prompt to set up autocompletion and add ng completion init#23060
Conversation
e7b8f72 to
70fc62a
Compare
|
/cc @twerske |
9298e4f to
42ca16c
Compare
| `Invalid config found at ${wksp.filePath}. \`extensions.cli\` should be an object.`, | ||
| ); | ||
| } | ||
| cli.completion ??= config as json.JsonObject; |
There was a problem hiding this comment.
This doesn't appear to be in the JSON schema.
This needs to be added
and also in the workspace schema and https://github.com/angular/angular-cli/blob/main/packages/angular/cli/lib/config/workspace-schema.jsonNote: at the moment options can be both at global and local level, currently there is no schema for global configs only.
There was a problem hiding this comment.
Thanks for pointing that out, added cli.completion.prompted (I'm not sure if we expect cli.completion to do anything)?
There was a problem hiding this comment.
I'm also wondering if we should have a more centralized workspace definition with some runtime component so we don't need to hard-code these paths? It would be very easy to forget them in the future.
| } | ||
|
|
||
| class CompletionInitCommandModule extends CommandModule implements CommandModuleImplementation { | ||
| command = 'init'; |
There was a problem hiding this comment.
Maybe instead of having a sub command, the prompt should be in the completion command?
There was a problem hiding this comment.
I see ng completion as responsible for generating the shell configuration for auto completion, while ng completion init is for setting up your .bashrc to use ng completion. Since these are two different actions, I think they should be different commands.
Considering that ng completion is something that users won't generally be calling themselves, I could see changing ng completion to initialize the setup by appending source <(ng completion --get-shell-config) or something if we want to view that part as more of an implementation detail. I could go either way whether --get-shell-config should be a flag on ng completion or a subcommand.
There was a problem hiding this comment.
I like the get-shell-config concept. Maybe since users will not need to use this command/option it can also be hidden both from help and docs (aio).
Nit: maybe get-completion-script would be a better name.
There was a problem hiding this comment.
I think I like ng completion being the one that users run to set up autocompletion and the ng completion some-flag-or-command being the one that is added to shell configs, since users care less about that. I'm don't feel very strongly about some-flag-or-command given that it still needs to be public API anyways just for people who customize their .bashrc setups heavily. I think I still slightly prefer a subcommand rather than a flag since it has a completely different use case and different outputs which would be a lot to change with a single flag. As for the subcommand name get-completion-script is a little long and redundant, but it's also not something that users regularly type, so we don't need to overthink it too much.
I guess where I'm landing is:
ng completionmodifies your.bashrcto set up autocompletion for you.ng completion get-completion-scriptprints the Yargs setup.
There was a problem hiding this comment.
@clydin suggested ng completion script, which I like since we don't need to repeat "completion" (and the "get" isn't that valuable either).
|
Using Maybe we should consider adding directly the script instead? |
4b795b1 to
9777d75
Compare
I was wondering about this too. My thought process was that having users inline Yargs output would be leaking an implementation detail. It becomes harder to change in the future (whether by us or Yargs itself), is less clear about its purpose, and harder to detect if it is already setup (need to search for Yargs output which more tightly couples us to it). Where did you get the 300ms number from? Is that some general understanding you have about the CLI startup time or something you've directly measured? |
This is something I measured, and is the time taken to execute the |
This fixes a few issues with the test: 1. The test accidentally used the real user's `$HOME` directory which led to confusing, non-hermetic failures. 2. The test had timeouts of 5 milliseconds, rather than the presumably intended 5 seconds. 3. `ng update` would always fail because there's no project. Changed to `ng update --help` which still skips the analytics prompt but doesn't require a complicated setup. 4. The test would end after seeing the analytics prompt and wouldn't bother accepting it. I added functionality to send data to stdin to accept the prompt which makes test logic simpler (no need to manually kill all processes or wait for a timeout), though I didn't add any new assertions that the CLI actually tracks / doesn't track correctly. Refs angular#23003.
Hmm, 300ms startup isn't great, but I'm also wary of exposing that shell configuration too broadly. I'm thinking we start with I'm also thinking that power users are likely willing to inline |
… modifying `.bashrc` files `ng completion` is changed to set up Angular CLI autocompletion for the current user by appending `source <(ng completion script)` to their `~/.bashrc`, `~/.bash_profile`, `~/.zshrc`, `~/.zsh_profile`, or `~/.profile`. The previous `ng completion` functionality (printing Yargs autocompletion shell script) is moved to `ng completion script` because most users won't need to worry about this, so we're prioritizing `ng completion` as the part most users will actually type. I couldn't find a good way of testing an error when writing to the `~/.bashrc` file. Since the CLI checks if it has access to the file first, that would usually fail in any circumstance when the file can't be written to. Things could change in between (user modifies file permissions or disk runs out of storage), but there's no easy hook to simulate this change in the e2e test. Refs angular#23003.
When the CLI is executed with any command, it will check if `ng completion script` is already included in the user's `~/.bashrc` file (or similar) and if not, ask the user if they would like it to be configured for them. The CLI checks any existing `~/.bashrc`, `~/.zshrc`, `~/.bash_profile`, `~/.zsh_profile`, and `~/.profile` files for `ng completion script`, and if that string is found for the current shell's configuration files, this prompt is skipped. If the user refuses the prompt, no action is taken and the CLI continues on the command the user originally requested. Refs angular#23003.
…letion and don't prompt again After the user rejects the autocompletion prompt or accepts and is successfully configured, the state is saved into the Angular CLI's global configuration. Before displaying the autocompletion prompt, this state is checked and the prompt is skipped if it was already shown. If the user accepts the prompt but the setup process fails, then the CLI will prompt again on the next execution, this gives users an opportunity to fix whatever issue they are encountering and try again. Refs angular#23003.
…ate` and `ng completion` commands `ng update` is most likely called when upgrading a project to the next version and users should be more concerned about their project than their personal terminal setup. `ng completion` is unconditionally setting up autocompletion, while `ng completion script` is getting the shell script for autocompletion setup. As a result, both of these don't benefit from a prompt and should be safe to skip it.
|
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. |
This updates the CLI to set up autocompletion for users automatically. Running
ng completionnow finds the relevant~/.bashrc,~/.zshrc, etc. file and adds the command to load Angular autocompletion for you, so users don't have to understand how to do it.ng completion scriptprints the actual shell script which needs to be evaluated in the shell (previously this was done byng completiondirectly).This also includes a prompt which is displayed to users on first command execution. After set up or if the user refuses the prompt, the CLI stores this state in the global workspace configuration and avoids prompting again. Users are expected to use
ng completionif they want to set up autocompletion after previously refusing the prompt. This hopefully provides a good middle ground where users who might not be aware of the feature can discover it without being too annoying to users who have custom shell setups or don't want autocompletion.The prompt is also skipped when running on CI, in a non-interactive terminal, when running
ng completionorng updatecommands, or if a run configuration for the current shell already hasng completion scriptin it (implying the user already set this up).End-to-end tests cover most of the flows, though there are a couple error conditions which are impractical to test. This required some modifications to test infrastructure to support passing stdin and environment variables in the right places. I also had to rewrite analytics tests to be more stable and not break from adding a new prompt.
Closes #23003.