Add bash completion for `get` and `set` commands #2411

nicolasbock · 2022-01-25T16:30:42Z

This change adds completion for the get and set commands.

Signed-off-by: Nicolas Bock nicolas.bock@canonical.com

codecov · 2022-01-25T16:50:21Z

Codecov Report

Merging #2411 (d9cbc38) into main (2f43d15) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2411      +/-   ##
==========================================
- Coverage   85.22%   85.18%   -0.05%     
==========================================
  Files         208      207       -1     
  Lines       10388    10433      +45     
==========================================
+ Hits         8853     8887      +34     
- Misses       1535     1546      +11

Impacted Files	Coverage Δ
...rc/platform/backends/qemu/qemu_virtual_machine.cpp	`73.68% <0.00%> (-4.01%)`	⬇️
src/client/cli/cmd/set.cpp	`88.88% <0.00%> (-3.97%)`	⬇️
...backends/qemu/linux/qemu_platform_detail_linux.cpp	`96.05% <0.00%> (-2.58%)`	⬇️
src/utils/settings.cpp	`85.86% <0.00%> (-1.78%)`	⬇️
src/sshfs_mount/sftp_server.cpp	`96.72% <0.00%> (-0.05%)`	⬇️
src/client/cli/cmd/register.cpp	`100.00% <0.00%> (ø)`
src/client/common/prompters.cpp	`100.00% <0.00%> (ø)`
include/multipass/cli/prompters.h	`100.00% <0.00%> (ø)`
src/platform/console/unix_terminal.h
src/platform/console/unix_terminal.cpp	`100.00% <0.00%> (+100.00%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f43d15...d9cbc38. Read the comment docs.

ricab · 2022-01-27T11:21:52Z

Hi @nicolasbock. Thanks for this! I have been longing for this completion for a while, but didn't have time to get to it myself. I haven't tried it yet, but it's very welcome.

There are some other changes in the works that will affect that help text, but we can blend. Adding to my TODO list to review (if no one else does first).

nicolasbock · 2022-01-27T13:22:43Z

Hi @ricab ! Thanks for the comments! If you point me towards these other changes I can modify the completions here to make them work with those changes.

ricab · 2022-01-27T15:52:09Z

Hey @nicolasbock, have a look at #2352. That's a little outdated now, I mean to push again soon.

The thing is, we're going through an overhaul of the settings, with the client now leaving daemon settings to the daemon. The client will no longer know or care what settings the daemon understands (some of them will soon be dynamic). Instead, it will simply ship get/set/keys requests over as needed. But we have yet to settle on the best way to interrogate settings keys. For now, when the daemon can't be reached, I just have a placeholder for its settings ('local.*) in the help text you're parsing.

I will take your PR into account when considering the best approach and I'd otherwise let it simmer for a little 🙂 But I won't forget it. Hope that's alright!

nicolasbock · 2022-01-27T23:52:01Z

Thanks for the additional info @ricab !

ricab · 2022-02-16T18:48:32Z

bors try

bors · 2022-02-16T20:00:03Z

try

Build failed:

macOS-x86_64

ricab

Hey @nicolasbock, circling back to this as it makes sense to include for the next release (even if it could need updating later).

Nice code 👍 only a few details inline.

ricab · 2022-02-16T19:59:46Z

completions/bash/multipass

+        local append_string=""
+        local -a settings
+
+        if [[ -v 1 ]]; then


This doesn't seem to cut it, it's not getting into the if, so no "=".

This is working for me. When I type

multipass set client.primary-na

and hit the tab key it completes to

multipass set client.primary-name=

Weird, it doesn't here: https://asciinema.org/a/ibIom1Ff3YNs2IrLvrPOfVSrr

I wonder what's affecting it. Trying with a plain function doesn't work either:

elsa@ricab-xps13:~$ test(){ > if [[ -v 1 ]]; then > echo yes > else > echo no > fi > } elsa@ricab-xps13:~$ test no elsa@ricab-xps13:~$ test asdf

This works though:

elsa@ricab-xps13:~$ test(){ if [[ -v toto ]]; then echo yes; else echo no; fi; } elsa@ricab-xps13:~$ test no elsa@ricab-xps13:~$ toto=asfd elsa@ricab-xps13:~$ test yes

Perhaps -v for argument numbers only works on very recent bash? I have 5.0.17.

("elsa" is just a sandbox account I use)

ricab · 2022-02-16T20:02:43Z

completions/bash/multipass

@@ -227,6 +242,9 @@ _multipass_complete()
            _multipass_aliases
            opts="${opts} ${multipass_aliases}"
        ;;
+        "get")
+            opts="${opts} --raw"


Ideally, this would only be added if not already used. But we can probably let it stick, as that's what we're doing for the likes of --help already. And it is not technically wrong to say --raw multiple times.

Fixed with

Suggested change

opts="${opts} --raw"

if [[ ! ${COMP_WORDS[*]} =~ --raw ]]; then

opts="${opts} --raw"

fi

ricab · 2022-02-16T20:07:47Z

completions/bash/multipass

@@ -339,6 +357,13 @@ _multipass_complete()
                # TODO: Do use spaces after the completion of an option starting with "--".
                compopt -o nospace
            ;;
+            "get")
+                _multipass_setting_keys


Both get and set both take a single key argument (for the time being, at least). It would be nice to complete that argument only if not yet used (so that we're not suggesting something that doesn't work):

$ multipass get client.primary-name local.driver Need exactly one setting key.

This change adds completion for the `get` and `set` commands. Signed-off-by: Nicolas Bock <nicolas.bock@canonical.com>

ricab

Suggesting we move a piece of code to deal with get case only once.

ricab · 2022-02-17T11:46:30Z

completions/bash/multipass

+        "get")
+            if [[ ! ${COMP_WORDS[*]} =~ --raw ]]; then
+                opts="${opts} --raw"
+            fi
+        ;;


Suggested change

"get")

if [[ ! ${COMP_WORDS[*]} =~ --raw ]]; then

opts="${opts} --raw"

fi

;;

ricab · 2022-02-17T11:47:27Z

completions/bash/multipass

@@ -339,6 +364,13 @@ _multipass_complete()
                # TODO: Do use spaces after the completion of an option starting with "--".
                compopt -o nospace
            ;;
+            "get")
+                _multipass_setting_keys
+            ;;


Suggested change

;;

if [[ ! ${COMP_WORDS[*]} =~ --raw ]]; then

opts="${opts} --raw"

fi

;;

ricab self-requested a review January 27, 2022 11:22

bors bot added a commit that referenced this pull request Feb 16, 2022

Try #2411:

7606831

ricab reviewed Feb 16, 2022

View reviewed changes

Add bash completion for get and set commands

d9cbc38

This change adds completion for the `get` and `set` commands. Signed-off-by: Nicolas Bock <nicolas.bock@canonical.com>

nicolasbock force-pushed the completions branch from 3d62fe6 to d9cbc38 Compare February 16, 2022 21:10

ricab reviewed Feb 17, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bash completion for `get` and `set` commands #2411

Add bash completion for `get` and `set` commands #2411

nicolasbock commented Jan 25, 2022

codecov bot commented Jan 25, 2022 •

edited

ricab commented Jan 27, 2022

nicolasbock commented Jan 27, 2022

ricab commented Jan 27, 2022

nicolasbock commented Jan 27, 2022

ricab commented Feb 16, 2022

bors bot commented Feb 16, 2022

ricab left a comment

ricab Feb 16, 2022

nicolasbock Feb 16, 2022

ricab Feb 17, 2022

ricab Feb 16, 2022

nicolasbock Feb 16, 2022

ricab Feb 16, 2022

nicolasbock Feb 16, 2022

ricab left a comment

ricab Feb 17, 2022

ricab Feb 17, 2022

Add bash completion for get and set commands #2411

Are you sure you want to change the base?

Add bash completion for get and set commands #2411

Conversation

nicolasbock commented Jan 25, 2022

codecov bot commented Jan 25, 2022 • edited

Codecov Report

ricab commented Jan 27, 2022

nicolasbock commented Jan 27, 2022

ricab commented Jan 27, 2022

nicolasbock commented Jan 27, 2022

ricab commented Feb 16, 2022

bors bot commented Feb 16, 2022

try

ricab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add bash completion for `get` and `set` commands #2411

Add bash completion for `get` and `set` commands #2411

codecov bot commented Jan 25, 2022 •

edited