Skip to content
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

add basic implementation of tab completion #663

Merged
merged 13 commits into from
Jun 12, 2020
Merged

add basic implementation of tab completion #663

merged 13 commits into from
Jun 12, 2020

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented Apr 18, 2020

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • The PR follows our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce?

feature

  • What is the current behavior?

#649

  • What is the new behavior?

add completion with TAB press. At the current time, the feature is very basic and only work for bash. I'm planning to add compatibility with fish and zsh. I'll also try to add the completion for lib and cores.

  • Does this PR introduce a breaking change?

No breaking change.
To try the feature it's required to run arduino-cli completion bash > completion.sh and then source completion.sh
photo_2020-04-18_23-57-29

  • Other information:

See how to contribute

@ubidefeo
Copy link

recent versions of zsh allow for bash completion sourcing
bashcompinit and a source should be issued in the shell session init

https://computingforgeeks.com/how-to-source-bash-completion-script-file-in-zsh/

@umbynos
Copy link
Contributor Author

umbynos commented Apr 19, 2020

image
fish sneak peek! It's also showing the description of the commands and subcommands.
But there's a problem: fish apparently does not support env variables with dash in the name. And unfortunately there are some env varibles with the dash in the name like __arduino-cli_comp_do_file_comp in the autogenerated file. I managed in "fixing" by changing - with _ manually in the file. but this solution is not feasible.

@umbynos
Copy link
Contributor Author

umbynos commented Apr 19, 2020

image
Forgot to mention that also flags completion should work fine. As well as flag and subcommand description (only supported by fish and zsh)

@umbynos umbynos marked this pull request as ready for review April 26, 2020 15:23
@umbynos
Copy link
Contributor Author

umbynos commented Apr 26, 2020

I think I succeed in fixing the problem with env vars naming and fish compatibility.
The PR for me Its ready. I've tried the completion with bash and fish. @ubidefeo or @rsora could you please try with zsh?

I also added a flag --no-description in order to disable description for commands and flags (it only works with fish).

To test it:

  • Bash
    ./arduino-cli completion bash > arduino-cli.sh and then source arduino-cli.sh
  • Fish
    ./arduino-cli completion fish > arduino-cli.fish and then source arduino-cli.fish
  • Zsh
    ./arduino-cli completion zsh > _arduino-cli.zsh and then source _arduino-cli.zsh

Sourcing a script in ~/.zshrc, ~/.bashrc it's not recommended, but it's only for testing purposes.

The proper way to do it is to place arduino-cli.fish to ~/.config/fish/completions/ for fish, copy arduino-cli.sh to /etc/bash_completion.d/ for bash. The only way to make it work for zsh for me was to source it in ~.zshrc.

I still have not added fqbn, cores, libraries completion. They require more work and I think I'll do it in a following PR. Is it ok for you?

I still have to update the documentation accordingly. I'll do it as soon as I have your opinion on the PR.
😄

@umbynos umbynos changed the title add basic implementation of bash completion add basic implementation of tab completion Apr 26, 2020
@ubidefeo
Copy link

ubidefeo commented May 4, 2020

Will not work under my zsh, @umbynos
let's look at it together when you're around

@umbynos
Copy link
Contributor Author

umbynos commented May 5, 2020

image
I've fixed also zsh completion. It was not working because [ and ] needed escaping and I had to remove the # in front of compinit.
Should work now ☕

@ericonr
Copy link

ericonr commented May 13, 2020

I had missed the notification for this one, this is awesome! Great work! I tested the initial functionality, it seemed to work really well!

I know the PR is still in progress, but it could be interesting to add some notes on how to use this, specially for people building for distribution (that way the generated completion files can be packaged already in the correct places).

The functionality I tested is already pretty great, perhaps the --fbqn part can be left to another PR, as you mentioned? I can try to help with testting and development for that one. My main shell is Fish, let me know if you have anything requiring testing!

@umbynos
Copy link
Contributor Author

umbynos commented May 13, 2020

Thank you! Currently I'm following this plan:

  1. Make the completion work for zsh (I still have some problems in finding where to put the completion file)
  2. Add testing for this functionality (https://brbsix.github.io/2015/11/29/accessing-tab-completion-programmatically-in-bash/)
  3. Learn something about Homebrew and find a way to generate the completion file during the installation and place it in the correct place (depends on the shell used)(https://docs.brew.sh/Shell-Completion).
  4. Maybe add a task in Taskfile to install the completion in the right place.
  5. Add proper documentation

I'll try to improve the completion (add --fqbn, cores and libs completion) with another PR when this one will be ready and merged. I will certainly ask for your help, thank you!

@ericonr
Copy link

ericonr commented May 13, 2020

  1. My distro seems to use /usr/share/zsh/site-functions/_alacritty for zsh completions. User generated ones probably go in ~/.local/share/zsh/site-functions, as long as they follow the logical paths.

  2. Probably makes sense to store the completions for all the shells that can be used in MacOS, I think.

  3. Seems reasonable!

@ubidefeo
Copy link

@ericonr
Apparently there are still some rough edges on Mac OS.
I for instance run Prezto and @umbynos and I haven't been able to make it work without sourcing the completion and its compdef line.
I confide in @umbynos to make it work :)

@umbynos
Copy link
Contributor Author

umbynos commented Jun 8, 2020

@per1234 when you have time, could you please give a look to the documentation i've added please? 😄

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice @umbynos! I gave it a quick try with bash and zsh and it worked great.

I only have a few nitpicky suggestions for the documentation.

umbynos and others added 4 commits June 9, 2020 09:38
fix typos and corrections

Co-authored-by: per1234 <accounts@perglass.com>
@umbynos umbynos requested review from per1234 and rsora June 10, 2020 09:05
Copy link
Contributor

@rsora rsora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with bash and zsh and it worked smoothly following the provided documentation.
Excellent!

@umbynos umbynos merged commit 15d35de into arduino:master Jun 12, 2020
umbynos added a commit that referenced this pull request Jun 17, 2020
* add basic implementation of bash completion

* update cobra to 1.0.0 because of spf13/cobra#1048

* add support for zsh and fish, improved help messages

* add flag ´--no-descriptions´ to disable automatic command description (for shells that supports it)

* fix fish not supporting env variables with dash in the name

* fixed zsh completion not working

* revert "#compdef" patch

* add check on --no-description flag
add comments regarding "hacks"
change Replacer with ReplaceAll (for readability)

* add docs

* Apply suggestions from code review

fix typos and corrections

Co-authored-by: per1234 <accounts@perglass.com>

* forgot a space

* fix fish docs

* add test for completion

Co-authored-by: per1234 <accounts@perglass.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants