-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(wrangler/cli): add @bomb.sh/tab completions
#11113
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e8a27f9 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
45886d3 to
e9c0d42
Compare
dario-piotrowicz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff, thanks so much @AmirSa12 🙏
I've left a comment regarding the completions setup, could you take a look? 😄
Also could you advise on how I can manually test this feature? 🙂
(is there a setup required, etc...)
packages/wrangler/src/complete.ts
Outdated
| @@ -0,0 +1,304 @@ | |||
| // NOTE: The provided option-value completions can be customized or removed as needed. | |||
| // NOTE: Descriptions and flags based on https://developers.cloudflare.com/workers/wrangler/commands/ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that taking the values from the docs and using them here is a bit problematic (as we'd also need to keep them in sync, etc...)
Could you try to instead use the experimental_getWranglerCommands function to dynamically get the information you need and setup the completions from that? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @dario-piotrowicz ! Thanks for your review!
Yes, I will try to use the experimental_getWranglerCommands and let you know how it works with that!
I'll keep you posted!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! thanks! 🫶
Thank you, @dario-piotrowicz! on top of this, if you have your own cli, you can have completions for that as well, and if your cli is executed through a package manager, and supports tab completions, lets assume wrangler does, the next example would also work: |
Thanks @AmirSa12 😄 So... if I understand correctly for |
@dario-piotrowicz, the whole tab package relies on shell scripts.. all the magic happens in the shell, so in-order to have the completions, we need a shell script ( think of it as a plugin ) to inject in our terminal. by installing tab, you are only injecting the shell script, in other ways this means you can directly do |
I see, great! thanks for the clarification 😄🙏 |
|
@dario-piotrowicz , I'm amazed by what I could achieve via the but one note, I noticed that, by using this function, we are only handling options that have explicit choices defined in the command args (like log-level) some option values (like |
|
Hey! I'd like to add some context and technical details regarding the approaches used in both this PR and #11637 On top of this, from a quality and reliability standpoint, tests can be added for this PR as well.
If this approach is still not desired, I recommend an alternative to be the cli tool wrangler is using (yargs) as it provides completions. however it’s important to note that Additionally, we're happy to add tests or address any missing features or gaps you think cc @dario-piotrowicz @NuroDev @ascorbic @dmmulroy wrangler.mp4 |
db5124a to
768ef7a
Compare
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
NuroDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // Handle shell completion requests | ||
| if (argv[0] === "complete") { | ||
| handleCompletion(argv.slice(1)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we can instead make this follow the existing command format / structure.
That way we can have the wrangler complete command itself have:
- Auto completion
- Suggestions for shells it supports
- A help menu
We can also reference this code from another PR on how to implement this.
Hey @NuroDev! Happy to hear that from you.
Sure, I'm adding tests, and also please do let me know if anything else is needed, will take care of that!
It does support completions for flags! for more context, please read this comment that I shared with Dario in the current PR. when using the |
Both options work (custom handlers would be fine), but for consistency, especially since flags like |
|
Hey @AmirSa12!
Perfect, thank you! I approved a CI run just so we can check all the tests you added worked and look good. There's 2 small code formatting errors that need fixing and that should be it. I'll create a PR to add documentation for this feature that'll fix the CI / Checks workflow.
Ah, my mistake. I missed that, sorry 😅
I agree with @dmmulroy that it would be amazing to get that added so we could also support command flags out the gate. So if you are able to add that please feel free as it would be amazing. But if any issues come up I am more than happy to land this first iteration PR and revisit that in a future PR. |
|
Hey @AmirSa12, is there anything else needed on this or do you need any help with anything? Happy to lend a hand to help get this over the line and merged 😄 |
|
hey @NuroDev, sorry for the inactivity, we've had some personal issues on the side, i'll look into this and see if i can move it forward! |
|
ok i had a look, i think as you suggested, maybe we can merge this PR and open another PR later that can add more It might take more work so splitting it up in another PR might be a quicker and cleaner way to do this. This way when @AmirSa12 is back, he can have a serious look at it too. Let me know what you think @NuroDev, and again, really sorry for the delay! |
This PR introduces tab completion functionality for the wrangler cli, improving developer experience by allowing shell(zsh, powershell, fish, bash) autocompletion for commands, options, values, and flags. This also comes with npm, pnpm, yarn and bun copmpletions! ( npm exec ... , pnpm ... ) With this PR, users can navigate available commands, options and values more efficiently and faster with speeding up workflow.
A small video of how this looks:
wrangler.mp4
wranglercloudflare-docs#27514Fixes #53