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

snapcraft: hardcode support for compopt to fix bash completion #630

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

simondeziel
Copy link
Member

@simondeziel simondeziel commented Nov 22, 2024

@kadinsayani found that when executed by snapd, the compopt support detection was broken.

@simondeziel
Copy link
Member Author

@kadinsayani here's what I've checked so far:

lxc completion bash > /tmp/orig
lxc completion bash | sed "$MONSTRUOSITY" > /tmp/new-inline

Which gives this minimal diff:

$ diff -Naur /tmp/orig /tmp/new-inline 
--- /tmp/orig	2024-11-21 20:24:06.523373035 -0500
+++ /tmp/new-inline	2024-11-21 20:42:49.076393122 -0500
@@ -23,7 +23,7 @@
     # Prepare the command to request completions for the program.
     # Calling ${words[0]} instead of directly lxc allows handling aliases
     args=("${words[@]:1}")
-    requestComp="${words[0]} __complete ${args[*]}"
+    requestComp="/snap/lxd/current/bin/lxc __complete ${args[*]}"
 
     lastParam=${words[$((${#words[@]}-1))]}
     lastChar=${lastParam:$((${#lastParam}-1)):1}
@@ -269,7 +269,7 @@
             desc=${comp#*$tab}
             comp=${comp%%$tab*}
 
-            # $COLUMNS stores the current shell width.
+            COLUMN="$(tput cols)"  # store the current shell width.
             # Remove an extra 4 because we add 2 spaces and 2 parentheses.
             maxdesclength=$(( COLUMNS - longest - 4 ))
 
@@ -330,9 +330,9 @@
 }
 
 if [[ "builtin" = "builtin" ]]; then
-    complete -o default -F __start_lxc lxc
+    complete -o default -F __start_lxc lxd.lxc lxc
 else
-    complete -o default -o nospace -F __start_lxc lxc
+    complete -o default -o nospace -F __start_lxc lxd.lxc lxc
 fi
 
 # ex: ts=4 sw=4 et filetype=sh

Reported-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Copy link
Contributor

@kadinsayani kadinsayani left a comment

Choose a reason for hiding this comment

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

I'm yet to test this with a snapcraft build but LGTM!

@tomponline
Copy link
Member

@kadinsayani @simondeziel there appears to be two different proposals for the CLI completion, this PR and #629

which one is the preferred approach?

@simondeziel
Copy link
Member Author

Dunno about the preferred way but I just tested this one and it works:

$ sudo snap install --dangerous lxd_0+git.55c4560d_amd64.snap
[sudo] password for sdeziel: 
2024-11-22T09:07:12-05:00 INFO Waiting for "snap.lxd.daemon.service" to stop.
lxd 0+git.55c4560d installed

$ sudo snap alias lxd.lxc lxc

$ lxc storage volume copy d<tab>
$ lxc storage volume copy default/  # autocompleted without any trailing " ", yay!

@kadinsayani
Copy link
Contributor

Dunno about the preferred way but I just tested this one and it works:

$ sudo snap install --dangerous lxd_0+git.55c4560d_amd64.snap
[sudo] password for sdeziel: 
2024-11-22T09:07:12-05:00 INFO Waiting for "snap.lxd.daemon.service" to stop.
lxd 0+git.55c4560d installed

$ sudo snap alias lxd.lxc lxc

$ lxc storage volume copy d<tab>
$ lxc storage volume copy default/  # autocompleted without any trailing " ", yay!

Amazing 🎉

@kadinsayani
Copy link
Contributor

kadinsayani commented Nov 22, 2024

@kadinsayani @simondeziel there appears to be two different proposals for the CLI completion, this PR and #629

which one is the preferred approach?

Let's go with Simon's approach, and keep the other potential solutions in mind if we face further issues with snapd's completion handling. For reference: canonical/lxd#14506. Another approach is to create our own completion command; also see https://github.com/spf13/cobra/blob/02326d52c0704d79169819698693b0698e748f55/bash_completionsV2.go.

@tomponline tomponline marked this pull request as ready for review November 22, 2024 14:56
@tomponline tomponline merged commit 87fd56f into canonical:latest-edge Nov 22, 2024
2 checks passed
@simondeziel simondeziel deleted the compopt-hardcode branch November 22, 2024 15:31
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.

3 participants