1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 00/11] Make auto completion better
  4. View patch

[libvirt] [PATCH 08/11] tools: Provide bash autompletion file

Michal Privoznik posted 11 patches 7 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Michal Privoznik 7 years, 6 months ago 
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 configure.ac               |  3 ++
 libvirt.spec.in            |  2 ++
 m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/Makefile.am          | 22 ++++++++++++--
 tools/bash-completion/vsh  | 73 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 172 insertions(+), 2 deletions(-)
 create mode 100644 m4/virt-bash-completion.m4
 create mode 100644 tools/bash-completion/vsh

diff --git a/configure.ac b/configure.ac
index b2d991c3b..9103612bb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR
 LIBVIRT_ARG_ATTR
 LIBVIRT_ARG_AUDIT
 LIBVIRT_ARG_AVAHI
+LIBVIRT_ARG_BASH_COMPLETION
 LIBVIRT_ARG_BLKID
 LIBVIRT_ARG_CAPNG
 LIBVIRT_ARG_CURL
@@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC
 LIBVIRT_CHECK_ATTR
 LIBVIRT_CHECK_AUDIT
 LIBVIRT_CHECK_AVAHI
+LIBVIRT_CHECK_BASH_COMPLETION
 LIBVIRT_CHECK_BLKID
 LIBVIRT_CHECK_CAPNG
 LIBVIRT_CHECK_CURL
@@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR
 LIBVIRT_RESULT_ATTR
 LIBVIRT_RESULT_AUDIT
 LIBVIRT_RESULT_AVAHI
+LIBVIRT_RESULT_BASH_COMPLETION
 LIBVIRT_RESULT_BLKID
 LIBVIRT_RESULT_CAPNG
 LIBVIRT_RESULT_CURL
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b00689cab..67bbd128c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -2043,6 +2043,8 @@ exit 0
 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
 %{_datadir}/systemtap/tapset/libvirt_functions.stp
 
+%{_datadir}/bash-completion/completions/vsh
+
 
 %if %{with_systemd}
 %{_unitdir}/libvirt-guests.service
diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4
new file mode 100644
index 000000000..e1ef58740
--- /dev/null
+++ b/m4/virt-bash-completion.m4
@@ -0,0 +1,74 @@
+dnl Bash completion support
+dnl
+dnl Copyright (C) 2017 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl <http://www.gnu.org/licenses/>.
+dnl
+dnl Inspired by libguestfs code.
+dnl
+
+AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[
+  LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0])
+  LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR],
+                   [directory containing bash completions scripts],
+                   [check])
+])
+
+AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [
+  AC_REQUIRE([LIBVIRT_CHECK_READLINE])
+
+  if test "x$with_readline" != "xyes" ; then
+    if test "x$with_bash_completion" != "xyes" ; then
+      with_bash_completion=no
+    else
+      AC_MSG_ERROR([readline is required for bash completion support])
+    fi
+  else
+    if test "x$with_bash_completion" = "xcheck" ; then
+      with_bash_completion=yes
+    fi
+  fi
+
+  LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0])
+
+  if test "x$with_bash_completion" = "xyes" ; then
+    if test "x$with_bash_completions_dir" = "xcheck"; then
+      AC_MSG_CHECKING([for bash-completions directory])
+      BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)"
+      AC_MSG_RESULT([$BASH_COMPLETIONS_DIR])
+
+      dnl Replace bash completions's exec_prefix with our own.
+      dnl Note that ${exec_prefix} is kept verbatim at this point in time,
+      dnl and will only be expanded later, when make is called: this makes
+      dnl it possible to override such prefix at compilation or installation
+      dnl time
+      bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)"
+      if test "x$bash_completions_prefix" = "x" ; then
+        bash_completions_prefix="/usr"
+      fi
+
+      BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
+    elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then
+      AC_MSG_ERROR([bash-completions-dir must be used only with valid path])
+    else
+      BASH_COMPLETIONS_DIR=$with_bash_completions_dir
+    fi
+    AC_SUBST([BASH_COMPLETIONS_DIR])
+  fi
+])
+
+AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[
+  LIBVIRT_RESULT_LIB([BASH_COMPLETION])
+])
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 7513a73ac..34a81e69c 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -66,6 +66,7 @@ EXTRA_DIST = \
 	libvirt-guests.sysconf \
 	virt-login-shell.conf \
 	virsh-edit.c \
+	bash-completion/vsh \
 	$(PODFILES) \
 	$(MANINFILES) \
 	$(NULL)
@@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)"
 		< $< > $@-t && \
 	mv $@-t $@
 
-install-data-local: install-init install-systemd install-nss
+install-data-local: install-init install-systemd install-nss \
+	install-bash-completion
 
-uninstall-local: uninstall-init uninstall-systemd uninstall-nss
+uninstall-local: uninstall-init uninstall-systemd uninstall-nss \
+	uninstall-bash-completion
 
 install-sysconfig:
 	$(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig
@@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status
 	    mv $@-t $@
 
 
+if WITH_BASH_COMPLETION
+install-bash-completion:
+	$(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
+	$(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
+		"$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
+
+uninstall-bash-completion:
+	rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh
+	rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||:
+else ! WITH_BASH_COMPLETION
+install-bash-completion:
+uninstall-bash-completion:
+endif ! WITH_BASH_COMPLETION
+
+
 EXTRA_DIST += \
 	wireshark/util/genxdrstub.pl \
 	wireshark/util/make-dissector-reg
diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh
new file mode 100644
index 000000000..0c923c8b5
--- /dev/null
+++ b/tools/bash-completion/vsh
@@ -0,0 +1,73 @@
+#
+# virsh & virt-admin completion command
+#
+
+_vsh_complete()
+{
+    local words cword c=0 i=0 cur RO URI CMDLINE INPUT A
+
+    # Here, $COMP_WORDS is an array of words on the bash
+    # command line that user wants to complete. However, when
+    # parsing command line, the default set of word breaks is
+    # applied. This doesn't work for us as it mangles libvirt
+    # arguments, e.g. connection URI (with the default set it's
+    # split into multiple items within the array). Fortunately,
+    # there's a fixup function for the array.
+    _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword
+    COMP_WORDS=( "${words[@]}" )
+    COMP_CWORD=${cword}
+    cur=${COMP_WORDS[$COMP_CWORD]}
+
+    # See what URI is user trying to connect to and if they are
+    # connecting RO. Honour that.
+    while [ $c -le $COMP_CWORD ]; do
+        word="${COMP_WORDS[c]}"
+        case "$word" in
+            -r|--readonly) RO=1 ;;
+            -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;;
+            *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;;
+        esac
+        c=$((++c))
+    done
+
+    CMDLINE=
+    if [ -n "${RO}" ]; then
+        CMDLINE="${CMDLINE} -r"
+    fi
+    if [ -n "${URI}" ]; then
+        CMDLINE="${CMDLINE} -c ${URI}"
+    fi
+
+    INPUT="${COMP_WORDS[@]:$i}"
+
+    # Uncomment these lines for easy debug.
+#    echo;
+#    echo "RO=${flag_ro}";
+#    echo "URI=${URI}";
+#    echo "CMDLINE=${CMDLINE}";
+#    echo "INPUT[${#INPUT[@]}]=**${INPUT}**";
+#    echo "cur=${cur}";
+#    echo;
+#    return 0;
+
+    # Small shortcut here. According to manpage:
+    # When the function is executed, the first argument ($1) is
+    # the name of the command whose arguments are being
+    # completed.
+    # Therefore, we might just run $1.
+    A=($($1 ${CMDLINE} complete "${INPUT}" 2>/dev/null))
+
+    # If our 'complete' command hasn't offered anything offer
+    # filedir completion.
+    if [ ${#A[@]} -gt 0 ]; then
+        COMPREPLY=($(compgen -W "${A[*]%--}" -- ${cur}))
+    else
+        _filedir
+    fi
+    __ltrim_colon_completions "$cur"
+    return 0
+} &&
+complete -F _vsh_complete virsh &&
+complete -F _vsh_complete virt-admin
+
+# vim: ft=sh:et:ts=4:sw=4:tw=80
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Martin Kletzander 7 years, 6 months ago 
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> configure.ac               |  3 ++
> libvirt.spec.in            |  2 ++
> m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++
> tools/Makefile.am          | 22 ++++++++++++--
> tools/bash-completion/vsh  | 73 +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 172 insertions(+), 2 deletions(-)
> create mode 100644 m4/virt-bash-completion.m4
> create mode 100644 tools/bash-completion/vsh
>
>diff --git a/configure.ac b/configure.ac
>index b2d991c3b..9103612bb 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR
> LIBVIRT_ARG_ATTR
> LIBVIRT_ARG_AUDIT
> LIBVIRT_ARG_AVAHI
>+LIBVIRT_ARG_BASH_COMPLETION
> LIBVIRT_ARG_BLKID
> LIBVIRT_ARG_CAPNG
> LIBVIRT_ARG_CURL
>@@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC
> LIBVIRT_CHECK_ATTR
> LIBVIRT_CHECK_AUDIT
> LIBVIRT_CHECK_AVAHI
>+LIBVIRT_CHECK_BASH_COMPLETION
> LIBVIRT_CHECK_BLKID
> LIBVIRT_CHECK_CAPNG
> LIBVIRT_CHECK_CURL
>@@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR
> LIBVIRT_RESULT_ATTR
> LIBVIRT_RESULT_AUDIT
> LIBVIRT_RESULT_AVAHI
>+LIBVIRT_RESULT_BASH_COMPLETION
> LIBVIRT_RESULT_BLKID
> LIBVIRT_RESULT_CAPNG
> LIBVIRT_RESULT_CURL
>diff --git a/libvirt.spec.in b/libvirt.spec.in
>index b00689cab..67bbd128c 100644
>--- a/libvirt.spec.in
>+++ b/libvirt.spec.in
>@@ -2043,6 +2043,8 @@ exit 0
> %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
> %{_datadir}/systemtap/tapset/libvirt_functions.stp
>
>+%{_datadir}/bash-completion/completions/vsh
>+
>
> %if %{with_systemd}
> %{_unitdir}/libvirt-guests.service
>diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4
>new file mode 100644
>index 000000000..e1ef58740
>--- /dev/null
>+++ b/m4/virt-bash-completion.m4
>@@ -0,0 +1,74 @@
>+dnl Bash completion support
>+dnl
>+dnl Copyright (C) 2017 Red Hat, Inc.
>+dnl
>+dnl This library is free software; you can redistribute it and/or
>+dnl modify it under the terms of the GNU Lesser General Public
>+dnl License as published by the Free Software Foundation; either
>+dnl version 2.1 of the License, or (at your option) any later version.
>+dnl
>+dnl This library is distributed in the hope that it will be useful,
>+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
>+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+dnl Lesser General Public License for more details.
>+dnl
>+dnl You should have received a copy of the GNU Lesser General Public
>+dnl License along with this library.  If not, see
>+dnl <http://www.gnu.org/licenses/>.
>+dnl
>+dnl Inspired by libguestfs code.
>+dnl
>+
>+AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[
>+  LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0])
>+  LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR],
>+                   [directory containing bash completions scripts],
>+                   [check])
>+])
>+
>+AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [
>+  AC_REQUIRE([LIBVIRT_CHECK_READLINE])
>+
>+  if test "x$with_readline" != "xyes" ; then
>+    if test "x$with_bash_completion" != "xyes" ; then
>+      with_bash_completion=no
>+    else
>+      AC_MSG_ERROR([readline is required for bash completion support])
>+    fi
>+  else
>+    if test "x$with_bash_completion" = "xcheck" ; then
>+      with_bash_completion=yes
>+    fi

You should change the "check" to "yes" only after everything below succeeded.

>+  fi
>+
>+  LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0])
>+
>+  if test "x$with_bash_completion" = "xyes" ; then
>+    if test "x$with_bash_completions_dir" = "xcheck"; then
>+      AC_MSG_CHECKING([for bash-completions directory])
>+      BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)"
>+      AC_MSG_RESULT([$BASH_COMPLETIONS_DIR])
>+
>+      dnl Replace bash completions's exec_prefix with our own.
>+      dnl Note that ${exec_prefix} is kept verbatim at this point in time,
>+      dnl and will only be expanded later, when make is called: this makes
>+      dnl it possible to override such prefix at compilation or installation
>+      dnl time
>+      bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)"
>+      if test "x$bash_completions_prefix" = "x" ; then
>+        bash_completions_prefix="/usr"
>+      fi
>+
>+      BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
>+    elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then
>+      AC_MSG_ERROR([bash-completions-dir must be used only with valid path])

Otherwise you can error out here ^^ even when nobody requested anything.

>+    else
>+      BASH_COMPLETIONS_DIR=$with_bash_completions_dir
>+    fi
>+    AC_SUBST([BASH_COMPLETIONS_DIR])
>+  fi
>+])
>+
>+AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[
>+  LIBVIRT_RESULT_LIB([BASH_COMPLETION])
>+])
>diff --git a/tools/Makefile.am b/tools/Makefile.am
>index 7513a73ac..34a81e69c 100644
>--- a/tools/Makefile.am
>+++ b/tools/Makefile.am
>@@ -66,6 +66,7 @@ EXTRA_DIST = \
> 	libvirt-guests.sysconf \
> 	virt-login-shell.conf \
> 	virsh-edit.c \
>+	bash-completion/vsh \
> 	$(PODFILES) \
> 	$(MANINFILES) \
> 	$(NULL)
>@@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)"
> 		< $< > $@-t && \
> 	mv $@-t $@
>
>-install-data-local: install-init install-systemd install-nss
>+install-data-local: install-init install-systemd install-nss \
>+	install-bash-completion
>
>-uninstall-local: uninstall-init uninstall-systemd uninstall-nss
>+uninstall-local: uninstall-init uninstall-systemd uninstall-nss \
>+	uninstall-bash-completion
>
> install-sysconfig:
> 	$(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig
>@@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status
> 	    mv $@-t $@
>
>
>+if WITH_BASH_COMPLETION
>+install-bash-completion:
>+	$(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
>+	$(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
>+		"$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>+
>+uninstall-bash-completion:
>+	rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh
>+	rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||:
>+else ! WITH_BASH_COMPLETION
>+install-bash-completion:
>+uninstall-bash-completion:
>+endif ! WITH_BASH_COMPLETION
>+
>+
> EXTRA_DIST += \
> 	wireshark/util/genxdrstub.pl \
> 	wireshark/util/make-dissector-reg
>diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh
>new file mode 100644
>index 000000000..0c923c8b5
>--- /dev/null
>+++ b/tools/bash-completion/vsh
>@@ -0,0 +1,73 @@
>+#
>+# virsh & virt-admin completion command
>+#
>+
>+_vsh_complete()
>+{
>+    local words cword c=0 i=0 cur RO URI CMDLINE INPUT A
>+
>+    # Here, $COMP_WORDS is an array of words on the bash
>+    # command line that user wants to complete. However, when
>+    # parsing command line, the default set of word breaks is
>+    # applied. This doesn't work for us as it mangles libvirt
>+    # arguments, e.g. connection URI (with the default set it's
>+    # split into multiple items within the array). Fortunately,
>+    # there's a fixup function for the array.
>+    _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword
>+    COMP_WORDS=( "${words[@]}" )
>+    COMP_CWORD=${cword}
>+    cur=${COMP_WORDS[$COMP_CWORD]}
>+
>+    # See what URI is user trying to connect to and if they are
>+    # connecting RO. Honour that.
>+    while [ $c -le $COMP_CWORD ]; do
>+        word="${COMP_WORDS[c]}"
>+        case "$word" in
>+            -r|--readonly) RO=1 ;;
>+            -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;;
>+            *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;;
>+        esac
>+        c=$((++c))
>+    done
>+
>+    CMDLINE=
>+    if [ -n "${RO}" ]; then
>+        CMDLINE="${CMDLINE} -r"
>+    fi
>+    if [ -n "${URI}" ]; then
>+        CMDLINE="${CMDLINE} -c ${URI}"
>+    fi
>+

When I see all the things you have to do here, wouldn't it be easier to have a
virsh 'option' rather than a 'command' so that we don't have to parse anything
twice and just circumvent the command execution in virsh itself?  You would just
run the same command with '-C' (for example) appended after the program name.

Otherwise looks OK, I would just like your input on it.

Should we have any tests for it? ;-) =D
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Michal Privoznik 7 years, 6 months ago 
On 11/08/2017 03:46 PM, Martin Kletzander wrote:
> On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> configure.ac               |  3 ++
>> libvirt.spec.in            |  2 ++
>> m4/virt-bash-completion.m4 | 74
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> tools/Makefile.am          | 22 ++++++++++++--
>> tools/bash-completion/vsh  | 73
>> +++++++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 172 insertions(+), 2 deletions(-)
>> create mode 100644 m4/virt-bash-completion.m4
>> create mode 100644 tools/bash-completion/vsh
>>
>> diff --git a/configure.ac b/configure.ac
>> index b2d991c3b..9103612bb 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR
>> LIBVIRT_ARG_ATTR
>> LIBVIRT_ARG_AUDIT
>> LIBVIRT_ARG_AVAHI
>> +LIBVIRT_ARG_BASH_COMPLETION
>> LIBVIRT_ARG_BLKID
>> LIBVIRT_ARG_CAPNG
>> LIBVIRT_ARG_CURL
>> @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC
>> LIBVIRT_CHECK_ATTR
>> LIBVIRT_CHECK_AUDIT
>> LIBVIRT_CHECK_AVAHI
>> +LIBVIRT_CHECK_BASH_COMPLETION
>> LIBVIRT_CHECK_BLKID
>> LIBVIRT_CHECK_CAPNG
>> LIBVIRT_CHECK_CURL
>> @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR
>> LIBVIRT_RESULT_ATTR
>> LIBVIRT_RESULT_AUDIT
>> LIBVIRT_RESULT_AVAHI
>> +LIBVIRT_RESULT_BASH_COMPLETION
>> LIBVIRT_RESULT_BLKID
>> LIBVIRT_RESULT_CAPNG
>> LIBVIRT_RESULT_CURL
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index b00689cab..67bbd128c 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -2043,6 +2043,8 @@ exit 0
>> %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
>> %{_datadir}/systemtap/tapset/libvirt_functions.stp
>>
>> +%{_datadir}/bash-completion/completions/vsh
>> +
>>
>> %if %{with_systemd}
>> %{_unitdir}/libvirt-guests.service
>> diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4
>> new file mode 100644
>> index 000000000..e1ef58740
>> --- /dev/null
>> +++ b/m4/virt-bash-completion.m4
>> @@ -0,0 +1,74 @@
>> +dnl Bash completion support
>> +dnl
>> +dnl Copyright (C) 2017 Red Hat, Inc.
>> +dnl
>> +dnl This library is free software; you can redistribute it and/or
>> +dnl modify it under the terms of the GNU Lesser General Public
>> +dnl License as published by the Free Software Foundation; either
>> +dnl version 2.1 of the License, or (at your option) any later version.
>> +dnl
>> +dnl This library is distributed in the hope that it will be useful,
>> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +dnl Lesser General Public License for more details.
>> +dnl
>> +dnl You should have received a copy of the GNU Lesser General Public
>> +dnl License along with this library.  If not, see
>> +dnl <http://www.gnu.org/licenses/>.
>> +dnl
>> +dnl Inspired by libguestfs code.
>> +dnl
>> +
>> +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[
>> +  LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion],
>> [check], [2.0])
>> +  LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR],
>> +                   [directory containing bash completions scripts],
>> +                   [check])
>> +])
>> +
>> +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [
>> +  AC_REQUIRE([LIBVIRT_CHECK_READLINE])
>> +
>> +  if test "x$with_readline" != "xyes" ; then
>> +    if test "x$with_bash_completion" != "xyes" ; then
>> +      with_bash_completion=no
>> +    else
>> +      AC_MSG_ERROR([readline is required for bash completion support])
>> +    fi
>> +  else
>> +    if test "x$with_bash_completion" = "xcheck" ; then
>> +      with_bash_completion=yes
>> +    fi
> 
> You should change the "check" to "yes" only after everything below
> succeeded.
> 
>> +  fi
>> +
>> +  LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0])
>> +
>> +  if test "x$with_bash_completion" = "xyes" ; then
>> +    if test "x$with_bash_completions_dir" = "xcheck"; then
>> +      AC_MSG_CHECKING([for bash-completions directory])
>> +      BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir
>> bash-completion)"
>> +      AC_MSG_RESULT([$BASH_COMPLETIONS_DIR])
>> +
>> +      dnl Replace bash completions's exec_prefix with our own.
>> +      dnl Note that ${exec_prefix} is kept verbatim at this point in
>> time,
>> +      dnl and will only be expanded later, when make is called: this
>> makes
>> +      dnl it possible to override such prefix at compilation or
>> installation
>> +      dnl time
>> +      bash_completions_prefix="$($PKG_CONFIG --variable=prefix
>> bash-completion)"
>> +      if test "x$bash_completions_prefix" = "x" ; then
>> +        bash_completions_prefix="/usr"
>> +      fi
>> +
>> +     
>> BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
>>
>> +    elif test "x$with_bash_completions_dir" = "xno" || test
>> "x$with_bash_completions_dir" = "xyes"; then
>> +      AC_MSG_ERROR([bash-completions-dir must be used only with valid
>> path])
> 
> Otherwise you can error out here ^^ even when nobody requested anything.
> 
>> +    else
>> +      BASH_COMPLETIONS_DIR=$with_bash_completions_dir
>> +    fi
>> +    AC_SUBST([BASH_COMPLETIONS_DIR])
>> +  fi
>> +])
>> +
>> +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[
>> +  LIBVIRT_RESULT_LIB([BASH_COMPLETION])
>> +])
>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>> index 7513a73ac..34a81e69c 100644
>> --- a/tools/Makefile.am
>> +++ b/tools/Makefile.am
>> @@ -66,6 +66,7 @@ EXTRA_DIST = \
>>     libvirt-guests.sysconf \
>>     virt-login-shell.conf \
>>     virsh-edit.c \
>> +    bash-completion/vsh \
>>     $(PODFILES) \
>>     $(MANINFILES) \
>>     $(NULL)
>> @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r
>> "$(PACKAGE)-$(VERSION)"
>>         < $< > $@-t && \
>>     mv $@-t $@
>>
>> -install-data-local: install-init install-systemd install-nss
>> +install-data-local: install-init install-systemd install-nss \
>> +    install-bash-completion
>>
>> -uninstall-local: uninstall-init uninstall-systemd uninstall-nss
>> +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \
>> +    uninstall-bash-completion
>>
>> install-sysconfig:
>>     $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig
>> @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in
>> $(top_builddir)/config.status
>>         mv $@-t $@
>>
>>
>> +if WITH_BASH_COMPLETION
>> +install-bash-completion:
>> +    $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
>> +    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
>> +        "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>> +
>> +uninstall-bash-completion:
>> +    rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh
>> +    rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||:
>> +else ! WITH_BASH_COMPLETION
>> +install-bash-completion:
>> +uninstall-bash-completion:
>> +endif ! WITH_BASH_COMPLETION
>> +
>> +
>> EXTRA_DIST += \
>>     wireshark/util/genxdrstub.pl \
>>     wireshark/util/make-dissector-reg
>> diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh
>> new file mode 100644
>> index 000000000..0c923c8b5
>> --- /dev/null
>> +++ b/tools/bash-completion/vsh
>> @@ -0,0 +1,73 @@
>> +#
>> +# virsh & virt-admin completion command
>> +#
>> +
>> +_vsh_complete()
>> +{
>> +    local words cword c=0 i=0 cur RO URI CMDLINE INPUT A
>> +
>> +    # Here, $COMP_WORDS is an array of words on the bash
>> +    # command line that user wants to complete. However, when
>> +    # parsing command line, the default set of word breaks is
>> +    # applied. This doesn't work for us as it mangles libvirt
>> +    # arguments, e.g. connection URI (with the default set it's
>> +    # split into multiple items within the array). Fortunately,
>> +    # there's a fixup function for the array.
>> +    _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword
>> +    COMP_WORDS=( "${words[@]}" )
>> +    COMP_CWORD=${cword}
>> +    cur=${COMP_WORDS[$COMP_CWORD]}
>> +
>> +    # See what URI is user trying to connect to and if they are
>> +    # connecting RO. Honour that.
>> +    while [ $c -le $COMP_CWORD ]; do
>> +        word="${COMP_WORDS[c]}"
>> +        case "$word" in
>> +            -r|--readonly) RO=1 ;;
>> +            -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;;
>> +            *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;;
>> +        esac
>> +        c=$((++c))
>> +    done
>> +
>> +    CMDLINE=
>> +    if [ -n "${RO}" ]; then
>> +        CMDLINE="${CMDLINE} -r"
>> +    fi
>> +    if [ -n "${URI}" ]; then
>> +        CMDLINE="${CMDLINE} -c ${URI}"
>> +    fi
>> +
> 
> When I see all the things you have to do here, wouldn't it be easier to
> have a
> virsh 'option' rather than a 'command' so that we don't have to parse
> anything
> twice and just circumvent the command execution in virsh itself?

Not really. That would mean parsing the command line in cmdComplete.
Which again might be incomplete and thus yet more code would be needed.
I don't really see a problem with this approach - now that the bash
script is written.

>  You
> would just
> run the same command with '-C' (for example) appended after the program
> name.

Yeah, there are dozen of other approaches. I've chosen this one. I'm
failing to see why one is better than another one.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Martin Kletzander 7 years, 6 months ago 
On Wed, Nov 08, 2017 at 04:09:10PM +0100, Michal Privoznik wrote:
>On 11/08/2017 03:46 PM, Martin Kletzander wrote:
>> On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>> configure.ac               |  3 ++
>>> libvirt.spec.in            |  2 ++
>>> m4/virt-bash-completion.m4 | 74
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>> tools/Makefile.am          | 22 ++++++++++++--
>>> tools/bash-completion/vsh  | 73
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> 5 files changed, 172 insertions(+), 2 deletions(-)
>>> create mode 100644 m4/virt-bash-completion.m4
>>> create mode 100644 tools/bash-completion/vsh
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index b2d991c3b..9103612bb 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR
>>> LIBVIRT_ARG_ATTR
>>> LIBVIRT_ARG_AUDIT
>>> LIBVIRT_ARG_AVAHI
>>> +LIBVIRT_ARG_BASH_COMPLETION
>>> LIBVIRT_ARG_BLKID
>>> LIBVIRT_ARG_CAPNG
>>> LIBVIRT_ARG_CURL
>>> @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC
>>> LIBVIRT_CHECK_ATTR
>>> LIBVIRT_CHECK_AUDIT
>>> LIBVIRT_CHECK_AVAHI
>>> +LIBVIRT_CHECK_BASH_COMPLETION
>>> LIBVIRT_CHECK_BLKID
>>> LIBVIRT_CHECK_CAPNG
>>> LIBVIRT_CHECK_CURL
>>> @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR
>>> LIBVIRT_RESULT_ATTR
>>> LIBVIRT_RESULT_AUDIT
>>> LIBVIRT_RESULT_AVAHI
>>> +LIBVIRT_RESULT_BASH_COMPLETION
>>> LIBVIRT_RESULT_BLKID
>>> LIBVIRT_RESULT_CAPNG
>>> LIBVIRT_RESULT_CURL
>>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>>> index b00689cab..67bbd128c 100644
>>> --- a/libvirt.spec.in
>>> +++ b/libvirt.spec.in
>>> @@ -2043,6 +2043,8 @@ exit 0
>>> %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
>>> %{_datadir}/systemtap/tapset/libvirt_functions.stp
>>>
>>> +%{_datadir}/bash-completion/completions/vsh
>>> +
>>>
>>> %if %{with_systemd}
>>> %{_unitdir}/libvirt-guests.service
>>> diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4
>>> new file mode 100644
>>> index 000000000..e1ef58740
>>> --- /dev/null
>>> +++ b/m4/virt-bash-completion.m4
>>> @@ -0,0 +1,74 @@
>>> +dnl Bash completion support
>>> +dnl
>>> +dnl Copyright (C) 2017 Red Hat, Inc.
>>> +dnl
>>> +dnl This library is free software; you can redistribute it and/or
>>> +dnl modify it under the terms of the GNU Lesser General Public
>>> +dnl License as published by the Free Software Foundation; either
>>> +dnl version 2.1 of the License, or (at your option) any later version.
>>> +dnl
>>> +dnl This library is distributed in the hope that it will be useful,
>>> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +dnl Lesser General Public License for more details.
>>> +dnl
>>> +dnl You should have received a copy of the GNU Lesser General Public
>>> +dnl License along with this library.  If not, see
>>> +dnl <http://www.gnu.org/licenses/>.
>>> +dnl
>>> +dnl Inspired by libguestfs code.
>>> +dnl
>>> +
>>> +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[
>>> +  LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion],
>>> [check], [2.0])
>>> +  LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR],
>>> +                   [directory containing bash completions scripts],
>>> +                   [check])
>>> +])
>>> +
>>> +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [
>>> +  AC_REQUIRE([LIBVIRT_CHECK_READLINE])
>>> +
>>> +  if test "x$with_readline" != "xyes" ; then
>>> +    if test "x$with_bash_completion" != "xyes" ; then
>>> +      with_bash_completion=no
>>> +    else
>>> +      AC_MSG_ERROR([readline is required for bash completion support])
>>> +    fi
>>> +  else
>>> +    if test "x$with_bash_completion" = "xcheck" ; then
>>> +      with_bash_completion=yes
>>> +    fi
>>
>> You should change the "check" to "yes" only after everything below
>> succeeded.
>>
>>> +  fi
>>> +
>>> +  LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0])
>>> +
>>> +  if test "x$with_bash_completion" = "xyes" ; then
>>> +    if test "x$with_bash_completions_dir" = "xcheck"; then
>>> +      AC_MSG_CHECKING([for bash-completions directory])
>>> +      BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir
>>> bash-completion)"
>>> +      AC_MSG_RESULT([$BASH_COMPLETIONS_DIR])
>>> +
>>> +      dnl Replace bash completions's exec_prefix with our own.
>>> +      dnl Note that ${exec_prefix} is kept verbatim at this point in
>>> time,
>>> +      dnl and will only be expanded later, when make is called: this
>>> makes
>>> +      dnl it possible to override such prefix at compilation or
>>> installation
>>> +      dnl time
>>> +      bash_completions_prefix="$($PKG_CONFIG --variable=prefix
>>> bash-completion)"
>>> +      if test "x$bash_completions_prefix" = "x" ; then
>>> +        bash_completions_prefix="/usr"
>>> +      fi
>>> +
>>> +     
>>> BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
>>>
>>> +    elif test "x$with_bash_completions_dir" = "xno" || test
>>> "x$with_bash_completions_dir" = "xyes"; then
>>> +      AC_MSG_ERROR([bash-completions-dir must be used only with valid
>>> path])
>>
>> Otherwise you can error out here ^^ even when nobody requested anything.
>>
>>> +    else
>>> +      BASH_COMPLETIONS_DIR=$with_bash_completions_dir
>>> +    fi
>>> +    AC_SUBST([BASH_COMPLETIONS_DIR])
>>> +  fi
>>> +])
>>> +
>>> +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[
>>> +  LIBVIRT_RESULT_LIB([BASH_COMPLETION])
>>> +])
>>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>>> index 7513a73ac..34a81e69c 100644
>>> --- a/tools/Makefile.am
>>> +++ b/tools/Makefile.am
>>> @@ -66,6 +66,7 @@ EXTRA_DIST = \
>>>     libvirt-guests.sysconf \
>>>     virt-login-shell.conf \
>>>     virsh-edit.c \
>>> +    bash-completion/vsh \
>>>     $(PODFILES) \
>>>     $(MANINFILES) \
>>>     $(NULL)
>>> @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r
>>> "$(PACKAGE)-$(VERSION)"
>>>         < $< > $@-t && \
>>>     mv $@-t $@
>>>
>>> -install-data-local: install-init install-systemd install-nss
>>> +install-data-local: install-init install-systemd install-nss \
>>> +    install-bash-completion
>>>
>>> -uninstall-local: uninstall-init uninstall-systemd uninstall-nss
>>> +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \
>>> +    uninstall-bash-completion
>>>
>>> install-sysconfig:
>>>     $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig
>>> @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in
>>> $(top_builddir)/config.status
>>>         mv $@-t $@
>>>
>>>
>>> +if WITH_BASH_COMPLETION
>>> +install-bash-completion:
>>> +    $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
>>> +    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
>>> +        "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>>> +
>>> +uninstall-bash-completion:
>>> +    rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh
>>> +    rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||:
>>> +else ! WITH_BASH_COMPLETION
>>> +install-bash-completion:
>>> +uninstall-bash-completion:
>>> +endif ! WITH_BASH_COMPLETION
>>> +
>>> +
>>> EXTRA_DIST += \
>>>     wireshark/util/genxdrstub.pl \
>>>     wireshark/util/make-dissector-reg
>>> diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh
>>> new file mode 100644
>>> index 000000000..0c923c8b5
>>> --- /dev/null
>>> +++ b/tools/bash-completion/vsh
>>> @@ -0,0 +1,73 @@
>>> +#
>>> +# virsh & virt-admin completion command
>>> +#
>>> +
>>> +_vsh_complete()
>>> +{
>>> +    local words cword c=0 i=0 cur RO URI CMDLINE INPUT A
>>> +
>>> +    # Here, $COMP_WORDS is an array of words on the bash
>>> +    # command line that user wants to complete. However, when
>>> +    # parsing command line, the default set of word breaks is
>>> +    # applied. This doesn't work for us as it mangles libvirt
>>> +    # arguments, e.g. connection URI (with the default set it's
>>> +    # split into multiple items within the array). Fortunately,
>>> +    # there's a fixup function for the array.
>>> +    _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword
>>> +    COMP_WORDS=( "${words[@]}" )
>>> +    COMP_CWORD=${cword}
>>> +    cur=${COMP_WORDS[$COMP_CWORD]}
>>> +
>>> +    # See what URI is user trying to connect to and if they are
>>> +    # connecting RO. Honour that.
>>> +    while [ $c -le $COMP_CWORD ]; do
>>> +        word="${COMP_WORDS[c]}"
>>> +        case "$word" in
>>> +            -r|--readonly) RO=1 ;;
>>> +            -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;;
>>> +            *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;;
>>> +        esac
>>> +        c=$((++c))
>>> +    done
>>> +
>>> +    CMDLINE=
>>> +    if [ -n "${RO}" ]; then
>>> +        CMDLINE="${CMDLINE} -r"
>>> +    fi
>>> +    if [ -n "${URI}" ]; then
>>> +        CMDLINE="${CMDLINE} -c ${URI}"
>>> +    fi
>>> +
>>
>> When I see all the things you have to do here, wouldn't it be easier to
>> have a
>> virsh 'option' rather than a 'command' so that we don't have to parse
>> anything
>> twice and just circumvent the command execution in virsh itself?
>
>Not really. That would mean parsing the command line in cmdComplete.
>Which again might be incomplete and thus yet more code would be needed.
>I don't really see a problem with this approach - now that the bash
>script is written.
>

No, there would be no cmdComplete.  And we already parse the whole thing anyway.

>>  You
>> would just
>> run the same command with '-C' (for example) appended after the program
>> name.
>
>Yeah, there are dozen of other approaches. I've chosen this one. I'm
>failing to see why one is better than another one.
>

Simplicity.

I'm not against this approach, I just wanted an open discussion about an idea.

>Michal
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Michal Privoznik 7 years, 6 months ago 
On 11/08/2017 04:18 PM, Martin Kletzander wrote:
> On Wed, Nov 08, 2017 at 04:09:10PM +0100, Michal Privoznik wrote:
>> On 11/08/2017 03:46 PM, Martin Kletzander wrote:
>>> On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>>> ---
>>>> configure.ac               |  3 ++
>>>> libvirt.spec.in            |  2 ++
>>>> m4/virt-bash-completion.m4 | 74
>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>> tools/Makefile.am          | 22 ++++++++++++--
>>>> tools/bash-completion/vsh  | 73
>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>> 5 files changed, 172 insertions(+), 2 deletions(-)
>>>> create mode 100644 m4/virt-bash-completion.m4
>>>> create mode 100644 tools/bash-completion/vsh
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index b2d991c3b..9103612bb 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR
>>>> LIBVIRT_ARG_ATTR
>>>> LIBVIRT_ARG_AUDIT
>>>> LIBVIRT_ARG_AVAHI
>>>> +LIBVIRT_ARG_BASH_COMPLETION
>>>> LIBVIRT_ARG_BLKID
>>>> LIBVIRT_ARG_CAPNG
>>>> LIBVIRT_ARG_CURL
>>>> @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC
>>>> LIBVIRT_CHECK_ATTR
>>>> LIBVIRT_CHECK_AUDIT
>>>> LIBVIRT_CHECK_AVAHI
>>>> +LIBVIRT_CHECK_BASH_COMPLETION
>>>> LIBVIRT_CHECK_BLKID
>>>> LIBVIRT_CHECK_CAPNG
>>>> LIBVIRT_CHECK_CURL
>>>> @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR
>>>> LIBVIRT_RESULT_ATTR
>>>> LIBVIRT_RESULT_AUDIT
>>>> LIBVIRT_RESULT_AVAHI
>>>> +LIBVIRT_RESULT_BASH_COMPLETION
>>>> LIBVIRT_RESULT_BLKID
>>>> LIBVIRT_RESULT_CAPNG
>>>> LIBVIRT_RESULT_CURL
>>>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>>>> index b00689cab..67bbd128c 100644
>>>> --- a/libvirt.spec.in
>>>> +++ b/libvirt.spec.in
>>>> @@ -2043,6 +2043,8 @@ exit 0
>>>> %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp
>>>> %{_datadir}/systemtap/tapset/libvirt_functions.stp
>>>>
>>>> +%{_datadir}/bash-completion/completions/vsh
>>>> +
>>>>
>>>> %if %{with_systemd}
>>>> %{_unitdir}/libvirt-guests.service
>>>> diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4
>>>> new file mode 100644
>>>> index 000000000..e1ef58740
>>>> --- /dev/null
>>>> +++ b/m4/virt-bash-completion.m4
>>>> @@ -0,0 +1,74 @@
>>>> +dnl Bash completion support
>>>> +dnl
>>>> +dnl Copyright (C) 2017 Red Hat, Inc.
>>>> +dnl
>>>> +dnl This library is free software; you can redistribute it and/or
>>>> +dnl modify it under the terms of the GNU Lesser General Public
>>>> +dnl License as published by the Free Software Foundation; either
>>>> +dnl version 2.1 of the License, or (at your option) any later version.
>>>> +dnl
>>>> +dnl This library is distributed in the hope that it will be useful,
>>>> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> +dnl Lesser General Public License for more details.
>>>> +dnl
>>>> +dnl You should have received a copy of the GNU Lesser General Public
>>>> +dnl License along with this library.  If not, see
>>>> +dnl <http://www.gnu.org/licenses/>.
>>>> +dnl
>>>> +dnl Inspired by libguestfs code.
>>>> +dnl
>>>> +
>>>> +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[
>>>> +  LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion],
>>>> [check], [2.0])
>>>> +  LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR],
>>>> +                   [directory containing bash completions scripts],
>>>> +                   [check])
>>>> +])
>>>> +
>>>> +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [
>>>> +  AC_REQUIRE([LIBVIRT_CHECK_READLINE])
>>>> +
>>>> +  if test "x$with_readline" != "xyes" ; then
>>>> +    if test "x$with_bash_completion" != "xyes" ; then
>>>> +      with_bash_completion=no
>>>> +    else
>>>> +      AC_MSG_ERROR([readline is required for bash completion support])
>>>> +    fi
>>>> +  else
>>>> +    if test "x$with_bash_completion" = "xcheck" ; then
>>>> +      with_bash_completion=yes
>>>> +    fi
>>>
>>> You should change the "check" to "yes" only after everything below
>>> succeeded.
>>>
>>>> +  fi
>>>> +
>>>> +  LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0])
>>>> +
>>>> +  if test "x$with_bash_completion" = "xyes" ; then
>>>> +    if test "x$with_bash_completions_dir" = "xcheck"; then
>>>> +      AC_MSG_CHECKING([for bash-completions directory])
>>>> +      BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir
>>>> bash-completion)"
>>>> +      AC_MSG_RESULT([$BASH_COMPLETIONS_DIR])
>>>> +
>>>> +      dnl Replace bash completions's exec_prefix with our own.
>>>> +      dnl Note that ${exec_prefix} is kept verbatim at this point in
>>>> time,
>>>> +      dnl and will only be expanded later, when make is called: this
>>>> makes
>>>> +      dnl it possible to override such prefix at compilation or
>>>> installation
>>>> +      dnl time
>>>> +      bash_completions_prefix="$($PKG_CONFIG --variable=prefix
>>>> bash-completion)"
>>>> +      if test "x$bash_completions_prefix" = "x" ; then
>>>> +        bash_completions_prefix="/usr"
>>>> +      fi
>>>> +
>>>> +     
>>>> BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
>>>>
>>>>
>>>> +    elif test "x$with_bash_completions_dir" = "xno" || test
>>>> "x$with_bash_completions_dir" = "xyes"; then
>>>> +      AC_MSG_ERROR([bash-completions-dir must be used only with valid
>>>> path])
>>>
>>> Otherwise you can error out here ^^ even when nobody requested anything.
>>>
>>>> +    else
>>>> +      BASH_COMPLETIONS_DIR=$with_bash_completions_dir
>>>> +    fi
>>>> +    AC_SUBST([BASH_COMPLETIONS_DIR])
>>>> +  fi
>>>> +])
>>>> +
>>>> +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[
>>>> +  LIBVIRT_RESULT_LIB([BASH_COMPLETION])
>>>> +])
>>>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>>>> index 7513a73ac..34a81e69c 100644
>>>> --- a/tools/Makefile.am
>>>> +++ b/tools/Makefile.am
>>>> @@ -66,6 +66,7 @@ EXTRA_DIST = \
>>>>     libvirt-guests.sysconf \
>>>>     virt-login-shell.conf \
>>>>     virsh-edit.c \
>>>> +    bash-completion/vsh \
>>>>     $(PODFILES) \
>>>>     $(MANINFILES) \
>>>>     $(NULL)
>>>> @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r
>>>> "$(PACKAGE)-$(VERSION)"
>>>>         < $< > $@-t && \
>>>>     mv $@-t $@
>>>>
>>>> -install-data-local: install-init install-systemd install-nss
>>>> +install-data-local: install-init install-systemd install-nss \
>>>> +    install-bash-completion
>>>>
>>>> -uninstall-local: uninstall-init uninstall-systemd uninstall-nss
>>>> +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \
>>>> +    uninstall-bash-completion
>>>>
>>>> install-sysconfig:
>>>>     $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig
>>>> @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in
>>>> $(top_builddir)/config.status
>>>>         mv $@-t $@
>>>>
>>>>
>>>> +if WITH_BASH_COMPLETION
>>>> +install-bash-completion:
>>>> +    $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)"
>>>> +    $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \
>>>> +        "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh"
>>>> +
>>>> +uninstall-bash-completion:
>>>> +    rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh
>>>> +    rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||:
>>>> +else ! WITH_BASH_COMPLETION
>>>> +install-bash-completion:
>>>> +uninstall-bash-completion:
>>>> +endif ! WITH_BASH_COMPLETION
>>>> +
>>>> +
>>>> EXTRA_DIST += \
>>>>     wireshark/util/genxdrstub.pl \
>>>>     wireshark/util/make-dissector-reg
>>>> diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh
>>>> new file mode 100644
>>>> index 000000000..0c923c8b5
>>>> --- /dev/null
>>>> +++ b/tools/bash-completion/vsh
>>>> @@ -0,0 +1,73 @@
>>>> +#
>>>> +# virsh & virt-admin completion command
>>>> +#
>>>> +
>>>> +_vsh_complete()
>>>> +{
>>>> +    local words cword c=0 i=0 cur RO URI CMDLINE INPUT A
>>>> +
>>>> +    # Here, $COMP_WORDS is an array of words on the bash
>>>> +    # command line that user wants to complete. However, when
>>>> +    # parsing command line, the default set of word breaks is
>>>> +    # applied. This doesn't work for us as it mangles libvirt
>>>> +    # arguments, e.g. connection URI (with the default set it's
>>>> +    # split into multiple items within the array). Fortunately,
>>>> +    # there's a fixup function for the array.
>>>> +    _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword
>>>> +    COMP_WORDS=( "${words[@]}" )
>>>> +    COMP_CWORD=${cword}
>>>> +    cur=${COMP_WORDS[$COMP_CWORD]}
>>>> +
>>>> +    # See what URI is user trying to connect to and if they are
>>>> +    # connecting RO. Honour that.
>>>> +    while [ $c -le $COMP_CWORD ]; do
>>>> +        word="${COMP_WORDS[c]}"
>>>> +        case "$word" in
>>>> +            -r|--readonly) RO=1 ;;
>>>> +            -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;;
>>>> +            *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break;
>>>> fi ;;
>>>> +        esac
>>>> +        c=$((++c))
>>>> +    done
>>>> +
>>>> +    CMDLINE=
>>>> +    if [ -n "${RO}" ]; then
>>>> +        CMDLINE="${CMDLINE} -r"
>>>> +    fi
>>>> +    if [ -n "${URI}" ]; then
>>>> +        CMDLINE="${CMDLINE} -c ${URI}"
>>>> +    fi
>>>> +
>>>
>>> When I see all the things you have to do here, wouldn't it be easier to
>>> have a
>>> virsh 'option' rather than a 'command' so that we don't have to parse
>>> anything
>>> twice and just circumvent the command execution in virsh itself?
>>
>> Not really. That would mean parsing the command line in cmdComplete.
>> Which again might be incomplete and thus yet more code would be needed.
>> I don't really see a problem with this approach - now that the bash
>> script is written.
>>
> 
> No, there would be no cmdComplete.  And we already parse the whole thing
> anyway.

How would it generate the list then? -C would need set some boolean flag
so that after parsing the vshReadlineCompletion() is called. So instead
of having the code in a separate cmd*() function it would be worked into
argv parsing. I don't really see the benefit.

> 
>>>   You
>>> would just
>>> run the same command with '-C' (for example) appended after the program
>>> name.
>>
>> Yeah, there are dozen of other approaches. I've chosen this one. I'm
>> failing to see why one is better than another one.
>>
> 
> Simplicity.

As I write above, while the bash script would be simpler, vsh's argv
parsing would be more complicated. Or am I missing something?

> 
> I'm not against this approach, I just wanted an open discussion about an
> idea.

Sure. I'm not saying that my approach is the best there is. It's just
that I understand this solution the most. Just take a look at our argv
parsing - I don't wan to touch that code with 10 feet pole unless I
really need to.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Eric Blake 7 years, 6 months ago 
On 11/08/2017 09:09 AM, Michal Privoznik wrote:
> On 11/08/2017 03:46 PM, Martin Kletzander wrote:
>> On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

>>
>> When I see all the things you have to do here, wouldn't it be easier to
>> have a
>> virsh 'option' rather than a 'command' so that we don't have to parse
>> anything
>> twice and just circumvent the command execution in virsh itself?
> 
> Not really. That would mean parsing the command line in cmdComplete.
> Which again might be incomplete and thus yet more code would be needed.
> I don't really see a problem with this approach - now that the bash
> script is written.
> 
>>   You
>> would just
>> run the same command with '-C' (for example) appended after the program
>> name.
> 
> Yeah, there are dozen of other approaches. I've chosen this one. I'm
> failing to see why one is better than another one.
> 

[I haven't read this thread closely yet, just adding a drive-by comment]

Several years ago, when autocompletion was attempted (and failed) as a
GSoC project, I had several ideas on how completion should work, and how
it should be tested.

Ideally, we need a new virsh command 'complete', which takes varargs,
and then performs completion based on those args.  So the bash script
for completion for this scenario:

$ virsh some-command --arg partial<TAB><TAB>

is as simple as having the completion function in bash call:

$ virsh complete some-command --arg partial<TAB><TAB>

The complete command in virsh should then know that it is performing
completion on some-command, and do enough arg parsing to determine how
to complete 'partial' in the current context of whatever argument
some-command would be parsing at that point.

If a user in bash backs up the cursor and types <TAB> somewhere other
than the end of the line, hopefully bash gives us enough hooks to call
'virsh complete args-up-to-TAB' by merely truncating whatever appears
beyond the point where the user attempted tab completion.

ALL the completion logic lives in virsh, nothing in bash - all bash has
to do is insert 'complete' and call into virsh.  That is, once you've
written the bash script, it should NEVER need future modification,
because any further improvements to completion will live in virsh
(whether you use virsh in interactive mode or in batch mode from the shell).

I don't know how much my original idea has carried over into your
proposal in this thread, but you may want to read the emails I posted
from the archives on the topic, for more ideas.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Michal Privoznik 7 years, 6 months ago 
On 11/08/2017 04:53 PM, Eric Blake wrote:
> On 11/08/2017 09:09 AM, Michal Privoznik wrote:
>> On 11/08/2017 03:46 PM, Martin Kletzander wrote:
>>> On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> 
>>>
>>> When I see all the things you have to do here, wouldn't it be easier to
>>> have a
>>> virsh 'option' rather than a 'command' so that we don't have to parse
>>> anything
>>> twice and just circumvent the command execution in virsh itself?
>>
>> Not really. That would mean parsing the command line in cmdComplete.
>> Which again might be incomplete and thus yet more code would be needed.
>> I don't really see a problem with this approach - now that the bash
>> script is written.
>>
>>>   You
>>> would just
>>> run the same command with '-C' (for example) appended after the program
>>> name.
>>
>> Yeah, there are dozen of other approaches. I've chosen this one. I'm
>> failing to see why one is better than another one.
>>
> 
> [I haven't read this thread closely yet, just adding a drive-by comment]
> 
> Several years ago, when autocompletion was attempted (and failed) as a
> GSoC project, I had several ideas on how completion should work, and how
> it should be tested.
> 
> Ideally, we need a new virsh command 'complete', which takes varargs,
> and then performs completion based on those args.  So the bash script
> for completion for this scenario:
> 
> $ virsh some-command --arg partial<TAB><TAB>
> 
> is as simple as having the completion function in bash call:
> 
> $ virsh complete some-command --arg partial<TAB><TAB>

This is excatly what my patches are doing. With one tiny difference. In
fact bash script calls:

  virsh complete "some-command --arg partial"

so that I can have cmdComplete which takes exactly one string argument.
Parsing of the string is then done within cmdComplete. I don't think
that we have variable args in virsh for commands, do we?

> 
> The complete command in virsh should then know that it is performing
> completion on some-command, and do enough arg parsing to determine how
> to complete 'partial' in the current context of whatever argument
> some-command would be parsing at that point.
> 
> If a user in bash backs up the cursor and types <TAB> somewhere other
> than the end of the line, hopefully bash gives us enough hooks to call
> 'virsh complete args-up-to-TAB' by merely truncating whatever appears
> beyond the point where the user attempted tab completion.
> 
> ALL the completion logic lives in virsh, nothing in bash - all bash has
> to do is insert 'complete' and call into virsh.  That is, once you've
> written the bash script, it should NEVER need future modification,
> because any further improvements to completion will live in virsh
> (whether you use virsh in interactive mode or in batch mode from the shell).

Correct. And again, my patches do this. For instance:

  virsh -r -c qemu+ssh://host/system domifaddr --domain<TAB><TAB>

becomes:

  virsh -r -c qemu+ssh://host/system complete "domifaddr --domain"

And it's the 'complete' command that is responsible for bringing up a
list of possible strings.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Eric Blake 7 years, 6 months ago 
On 11/08/2017 10:06 AM, Michal Privoznik wrote:

>> $ virsh complete some-command --arg partial<TAB><TAB>
> 
> This is excatly what my patches are doing. With one tiny difference. In
> fact bash script calls:
> 
>   virsh complete "some-command --arg partial"
> 
> so that I can have cmdComplete which takes exactly one string argument.

Ouch. That difference matters, and I don't think you want to do that.

We don't want to reimplement shell parsing; if the user does:

virsh snapshot-create-as 'domain with space' 'name with space'
'description with space', then my approach feeds those three arguments
as is (the virsh parser doesn't have to undo any shell quoting), while
your approach HAS to over-quote the original command line, then
reimplement shell quoting to remove those quotes in virsh, in order to
reconstruct the original line in spite of being temporarily squashed
through one argument.

> Parsing of the string is then done within cmdComplete. I don't think
> that we have variable args in virsh for commands, do we?

Yes, we do.  See 'virsh echo' for an example of its use.

> 
> Correct. And again, my patches do this. For instance:
> 
>   virsh -r -c qemu+ssh://host/system domifaddr --domain<TAB><TAB>
> 
> becomes:
> 
>   virsh -r -c qemu+ssh://host/system complete "domifaddr --domain"
> 
> And it's the 'complete' command that is responsible for bringing up a
> list of possible strings.

Ah, you are trying to put 'complete' after virsh-specific options (-r,
-c), but before the command (domifaddr).

And if properly implemented, you should be able to have:

virsh domif<TAB><TAB>

show 'domifaddr' (and any other commands starting with 'domif') (by
calling "virsh complete -- domif"),

virsh domifaddr <TAB><TAB>

show both a list of domain names AND a list of --options accepted by
domifaddr (by calling "virsh complete -- domifaddr ''"),

virsh domifaddr --<TAB><TAB>

show a list of long options for domifaddr (by calling "virsh complete --
domifaddr --") (yes, I'm specifically making sure the second '--'
doesn't get eaten by getopts, by inserting 'complete --' rather than
just 'complete'), and

virsh domifaddr a<TAB><TAB>

show a filtered list of just domain names starting with 'a' (by calling
"virsh complete domifaddr a").

That is, the completions should be context-sensitive based on how much
of the command line is present prior to the <TAB><TAB>, and should NOT
need the user to type an explicit --domain just to get domain completions.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Michal Privoznik 7 years, 6 months ago 
On 11/08/2017 05:26 PM, Eric Blake wrote:
> On 11/08/2017 10:06 AM, Michal Privoznik wrote:
> 
>>> $ virsh complete some-command --arg partial<TAB><TAB>
>>
>> This is excatly what my patches are doing. With one tiny difference. In
>> fact bash script calls:
>>
>>   virsh complete "some-command --arg partial"
>>
>> so that I can have cmdComplete which takes exactly one string argument.
> 
> Ouch. That difference matters, and I don't think you want to do that.
> 
> We don't want to reimplement shell parsing; if the user does:
> 
> virsh snapshot-create-as 'domain with space' 'name with space'
> 'description with space', then my approach feeds those three arguments
> as is (the virsh parser doesn't have to undo any shell quoting), while
> your approach HAS to over-quote the original command line, then
> reimplement shell quoting to remove those quotes in virsh, in order to
> reconstruct the original line in spite of being temporarily squashed
> through one argument.
> 
>> Parsing of the string is then done within cmdComplete. I don't think
>> that we have variable args in virsh for commands, do we?
> 
> Yes, we do.  See 'virsh echo' for an example of its use.

So while implementing this I realized it will not fly. At ARGV level it
is impossible to tell apart "--domain" and "--domain ". But the
difference is important, because in the first case we want to offer list
of all --options, while in the second case we want to call the --domain
completer. Okay, this might not be the best example because there's no
other option that shares the prefix, but for instance 'migrate' command
has:

    --timeout
    --timeout-suspend
    --timeout-postcopy

So if user types in:

  "virsh migrate --timeout"

we have to bring up list:

    "--timeout"
    "--timeout-suspend"
    "--timeout-postcopy"

and only after they type:

  "virsh migrate --timeout "

we can call --timeout completer. Since it's impossible to differentiate
these cases with VSH_OT_ARGV, I'm sticking with what I have now. Sure,
it might has its own problems but those can be solved later.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Eric Blake 7 years, 6 months ago 
On 11/10/2017 05:01 AM, Michal Privoznik wrote:

> 
> So while implementing this I realized it will not fly. At ARGV level it
> is impossible to tell apart "--domain" and "--domain ".

But it is not impossible to tell apart '"--domain"' (one arg) and
'"--domain" ""' (two args).  If you have trailing whitespace, then tack
on an empty argument.

> But the
> difference is important, because in the first case we want to offer list
> of all --options, while in the second case we want to call the --domain
> completer. 

Yes, the difference between one argument and two is significant - the
completion should always be done on the last argument, but the last
argument needs to be the empty string if it is not in the middle of a word.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/11] tools: Provide bash autompletion file
Posted by Michal Privoznik 7 years, 6 months ago 
On 11/08/2017 05:26 PM, Eric Blake wrote:
> On 11/08/2017 10:06 AM, Michal Privoznik wrote:
> 
>>> $ virsh complete some-command --arg partial<TAB><TAB>
>>
>> This is excatly what my patches are doing. With one tiny difference. In
>> fact bash script calls:
>>
>>   virsh complete "some-command --arg partial"
>>
>> so that I can have cmdComplete which takes exactly one string argument.
> 
> Ouch. That difference matters, and I don't think you want to do that.
> 
> We don't want to reimplement shell parsing; if the user does:
> 
> virsh snapshot-create-as 'domain with space' 'name with space'
> 'description with space', then my approach feeds those three arguments
> as is (the virsh parser doesn't have to undo any shell quoting), while
> your approach HAS to over-quote the original command line, then
> reimplement shell quoting to remove those quotes in virsh, in order to
> reconstruct the original line in spite of being temporarily squashed
> through one argument.

Right. On the other hand, who uses space in file names? ;-)

> 
>> Parsing of the string is then done within cmdComplete. I don't think
>> that we have variable args in virsh for commands, do we?
> 
> Yes, we do.  See 'virsh echo' for an example of its use.

Okay, I will look into this.

> 
>>
>> Correct. And again, my patches do this. For instance:
>>
>>   virsh -r -c qemu+ssh://host/system domifaddr --domain<TAB><TAB>
>>
>> becomes:
>>
>>   virsh -r -c qemu+ssh://host/system complete "domifaddr --domain"
>>
>> And it's the 'complete' command that is responsible for bringing up a
>> list of possible strings.
> 
> Ah, you are trying to put 'complete' after virsh-specific options (-r,
> -c), but before the command (domifaddr).

Yes.

> 
> And if properly implemented, you should be able to have:
> 
> virsh domif<TAB><TAB>
> 
> show 'domifaddr' (and any other commands starting with 'domif') (by
> calling "virsh complete -- domif"),

Yup. This works.

> 
> virsh domifaddr <TAB><TAB>
> 
> show both a list of domain names AND a list of --options accepted by
> domifaddr (by calling "virsh complete -- domifaddr ''"),

This is still WIP. As I mention in the cover letter, the parser is hard
to grasp for me and therefore this is yet to be implemented. Currently
all you get is list of --options. However, completers for --option work.
For instance:

virsh start --domain <TAB><TAB>

brings up list of domains to start.

> 
> virsh domifaddr --<TAB><TAB>
> 
> show a list of long options for domifaddr (by calling "virsh complete --
> domifaddr --") (yes, I'm specifically making sure the second '--'
> doesn't get eaten by getopts, by inserting 'complete --' rather than
> just 'complete'), and
> 
> virsh domifaddr a<TAB><TAB>
> 
> show a filtered list of just domain names starting with 'a' (by calling
> "virsh complete domifaddr a").
> 
> That is, the completions should be context-sensitive based on how much
> of the command line is present prior to the <TAB><TAB>, and should NOT
> need the user to type an explicit --domain just to get domain completions.
> 

Yeah, that's the idea. And I'm looking into the code, but it's not that
easy for me to understand it. So, if anybody wants to help, be my guest.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.