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

POC Dynamic shell completion for fqbn flag #1431

Merged
merged 5 commits into from
Sep 13, 2021
Merged

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented Sep 2, 2021

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)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • What kind of change does this PR introduce?

Dynamic shell completion

  • What is the current behavior?

Now the shell completion is only static and only works by completing sub-commands and flags hard-coded in the CLI code.

  • What is the new behavior?

With cobra 1.2.0 they added dynamic shell completion for nouns and for flags (see here and here).
The goal of this Proof Of Concept PR is to implement the dynamic completion for the arduino-cli compile --fqbn flag and sub-command, by retrieving the list of the fqbn (similar to board listall command).

umberto@XPS-15:~/Lavoro/arduino-cli$ ./arduino-cli compile --fqbn <TAB><TAB>
mbed_nano:nano33ble                 samd:arduino_zero_native            samd:mkrwan1300                     samd:mzero_pro_bl_dbg
mbed_nano:nanorp2040connect         samd:mkr1000                        samd:mkrwan1310                     samd:nano_33_iot
megaavr:nona4809                    samd:mkrfox1200                     samd:mkrwifi1010                    samd:tian
megaavr:uno2018                     samd:mkrgsm1400                     samd:mkrzero                        
samd:adafruit_circuitplayground_m0  samd:mkrnb1500                      samd:mzero_bl                       
samd:arduino_zero_edbg              samd:mkrvidor4000                   samd:mzero_pro_bl    

In order to make this happen, I bumped the cobra dependency to the latest version (1.2.1) in #1430
I used the newer GenBashCompletionV2 command to generate the completion for bash (to support dynamic completion)

  • Other information:

See how to contribute

@umbynos umbynos added type: enhancement Proposed improvement topic: CLI topic: code Related to content of the project itself labels Sep 2, 2021
@umbynos umbynos requested a review from a team September 2, 2021 17:08
@umbynos umbynos self-assigned this Sep 2, 2021
@umbynos umbynos force-pushed the umbynos/dynamic_completion branch 2 times, most recently from be3f4a5 to 59f1a35 Compare September 3, 2021 08:44
@@ -83,6 +84,10 @@ func NewCommand() *cobra.Command {
}

command.Flags().StringVarP(&fqbn, "fqbn", "b", "", tr("Fully Qualified Board Name, e.g.: arduino:avr:uno"))
command.MarkFlagRequired("fqbn")
Copy link
Contributor

Choose a reason for hiding this comment

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

This flag is not required due to the ability to define it in the sketch metadata via arduino-cli board attach:

$ ./arduino-cli version
arduino-cli.exe alpha Version: 0.19.0 Commit: 56419ecd Date: 2021-09-02T14:47:35Z

$ ./arduino-cli sketch new /tmp/FooSketch
Sketch created in: C:\Users\per\AppData\Local\Temp\FooSketch

$ ./arduino-cli board attach arduino:avr:uno /tmp/FooSketch
Selected fqbn: arduino:avr:uno

$ ./arduino-cli compile /tmp/FooSketch
Sketch uses 444 bytes (1%) of program storage space. Maximum is 32256 bytes.
Global variables use 9 bytes (0%) of dynamic memory, leaving 2039 bytes for local variables. Maximum is 2048 bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, I thought about it later... I just wanted to experiment with MarkFlagRequired function. We can use this somewhere else:
basically if you type completion <tab><tab> the completion suggest at first only the required flags, so the user is prompted to use them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW I reverted it 😄

@umbynos umbynos marked this pull request as ready for review September 3, 2021 14:07
@umbynos umbynos requested a review from a team September 3, 2021 14:30
@umbynos umbynos force-pushed the umbynos/dynamic_completion branch from 2cf6de9 to e0b6a80 Compare September 13, 2021 13:13
@umbynos umbynos force-pushed the umbynos/dynamic_completion branch from e0b6a80 to 53a8a36 Compare September 13, 2021 13:46
@umbynos umbynos merged commit 826adc5 into master Sep 13, 2021
@umbynos umbynos deleted the umbynos/dynamic_completion branch September 13, 2021 14:46
@rsora rsora added the topic: CLI Related to the command line interface label Sep 22, 2021
silvanocerza pushed a commit that referenced this pull request Sep 30, 2021
* make fqbn flag required, so that the completion suggests it when pressing <tab><tab>
for better explanation see https://github.com/spf13/cobra/blob/master/shell_completions.md#mark-flags-as-required

* use newer and supported bashv2 completion in order to have dynamic completion

* add dynamic completion for `compile -b` command to list available board

* Revert "make fqbn flag required, so that the completion suggests it when pressing <tab><tab>"

This reverts commit cc81265.

* make Internalization happy
@umbynos umbynos mentioned this pull request Oct 13, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: CLI Related to the command line interface topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants