All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] shell-completion: Add initial completion for kmod
@ 2013-07-30  6:48 Lucas De Marchi
  2013-07-30  6:48 ` [PATCH 2/3] shell-completion: Add kmod static-nodes Lucas De Marchi
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Lucas De Marchi @ 2013-07-30  6:48 UTC (permalink / raw)
  To: linux-modules

From: Lucas De Marchi <lucas.demarchi@intel.com>

Skeleton pulled from udevadm in systemd and adapted to kmod needs.
---
 NEWS                       |  2 +-
 shell-completion/bash/kmod | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 shell-completion/bash/kmod

diff --git a/NEWS b/NEWS
index 1dff366..491146d 100644
--- a/NEWS
+++ b/NEWS
@@ -6,7 +6,7 @@ kmod 15
 
 - New features:
 	- kmod static-nodes creates parent directories if given a -o option
-	- kmod binary doesn't statically link to libkmod
+	- kmod binary statically link to libkmod
 
 kmod 14
 =======
diff --git a/shell-completion/bash/kmod b/shell-completion/bash/kmod
new file mode 100644
index 0000000..81dbf46
--- /dev/null
+++ b/shell-completion/bash/kmod
@@ -0,0 +1,53 @@
+# kmod completion                                          -*- shell-script -*-
+#
+# This file is part of systemd.
+#
+# Copyright 2010 Ran Benita
+# Copyright (C) 2013  Intel Corporation. All rights reserved.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+#
+# systemd is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with systemd; If not, see <http://www.gnu.org/licenses/>.
+
+__contains_word () {
+    local word=$1; shift
+    for w in $*; do [[ $w = $word ]] && return 0; done
+    return 1
+}
+
+_kmod() {
+    local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
+    local verb comps
+
+    local -A VERBS=(
+	[HELP]='help'
+	[LIST]='list'
+	[STATIC-NODES]='static-nodes'
+    )
+
+    for ((i=0; $i <= $COMP_CWORD; i++)); do
+        if __contains_word "${COMP_WORDS[i]}" ${VERBS[*]} &&
+            ! __contains_word "${COMP_WORDS[i-1]}" ${OPTS[ARG]}; then
+            verb=${COMP_WORDS[i]}
+            break
+        fi
+    done
+
+    if [[ -z $verb ]]; then
+	comps=${VERBS[*]}
+    fi
+
+    COMPREPLY=( $(compgen -W '$comps' -- "$cur" ) )
+    return 0
+}
+
+complete -F _kmod kmod
-- 
1.8.3.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] shell-completion: Add kmod static-nodes
  2013-07-30  6:48 [PATCH 1/3] shell-completion: Add initial completion for kmod Lucas De Marchi
@ 2013-07-30  6:48 ` Lucas De Marchi
  2013-07-30  6:48 ` [PATCH 3/3] build: Install bash completion data Lucas De Marchi
  2013-07-30 13:17 ` [PATCH 1/3] shell-completion: Add initial completion for kmod Dave Reisner
  2 siblings, 0 replies; 7+ messages in thread
From: Lucas De Marchi @ 2013-07-30  6:48 UTC (permalink / raw)
  To: linux-modules

From: Lucas De Marchi <lucas.demarchi@intel.com>

---
 shell-completion/bash/kmod | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/shell-completion/bash/kmod b/shell-completion/bash/kmod
index 81dbf46..2409eda 100644
--- a/shell-completion/bash/kmod
+++ b/shell-completion/bash/kmod
@@ -24,6 +24,28 @@ __contains_word () {
     return 1
 }
 
+_kmod_static_nodes () {
+    local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
+    local opts='-o --output -f --format -h --help'
+
+    echo "cur='$cur' prev='$prev'" >> /tmp/a.txt
+    case $prev in
+	'-o' | '--output')
+	    compopt -o filenames
+	    COMPREPLY=( $(compgen -f -- "$cur") )
+	    ;;
+	'-f' | '--format')
+	    COMPREPLY=( $(compgen -W 'human tmpfiles devname' -- "$cur") )
+	    ;;
+	'-'*)
+	    COMPREPLY=()
+	    ;;
+	*)
+	    COMPREPLY=( $(compgen -W "$opts" -- "$cur") )
+	    ;;
+    esac
+}
+
 _kmod() {
     local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
     local verb comps
@@ -44,6 +66,11 @@ _kmod() {
 
     if [[ -z $verb ]]; then
 	comps=${VERBS[*]}
+    elif __contains_word "$verb" ${VERBS[STATIC-NODES]}; then
+	_kmod_static_nodes
+	return 0
+    else
+	comps=''
     fi
 
     COMPREPLY=( $(compgen -W '$comps' -- "$cur" ) )
-- 
1.8.3.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] build: Install bash completion data
  2013-07-30  6:48 [PATCH 1/3] shell-completion: Add initial completion for kmod Lucas De Marchi
  2013-07-30  6:48 ` [PATCH 2/3] shell-completion: Add kmod static-nodes Lucas De Marchi
@ 2013-07-30  6:48 ` Lucas De Marchi
  2013-07-30 13:17 ` [PATCH 1/3] shell-completion: Add initial completion for kmod Dave Reisner
  2 siblings, 0 replies; 7+ messages in thread
From: Lucas De Marchi @ 2013-07-30  6:48 UTC (permalink / raw)
  To: linux-modules

From: Lucas De Marchi <lucas.demarchi@intel.com>

---
 Makefile.am  |  7 ++++++-
 configure.ac | 12 +++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 57b7372..91a766a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -89,6 +89,10 @@ pkgconfig_DATA = libkmod/libkmod.pc
 EXTRA_DIST += libkmod/libkmod.pc.in
 CLEANFILES += libkmod/libkmod.pc
 
+bashcompletiondir=@bashcompletiondir@
+dist_bashcompletion_DATA = \
+	shell-completion/bash/kmod
+
 install-exec-hook:
 	if test "$(libdir)" != "$(rootlibdir)"; then \
 		$(MKDIR_P) $(DESTDIR)$(rootlibdir) && \
@@ -207,7 +211,8 @@ testsuite-distclean:
 DISTCLEAN_LOCAL_HOOKS += testsuite-distclean
 EXTRA_DIST += testsuite/rootfs-pristine
 
-DISTCHECK_CONFIGURE_FLAGS=--enable-gtk-doc --sysconfdir=/etc --with-zlib
+DISTCHECK_CONFIGURE_FLAGS=--enable-gtk-doc --sysconfdir=/etc --with-zlib \
+	--with-bashcompletiondir=$$dc_install_base/$(bashcompletiondir)
 
 distclean-local: $(DISTCLEAN_LOCAL_HOOKS)
 
diff --git a/configure.ac b/configure.ac
index 194dfab..15c69d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,15 @@ AS_IF([test "x$with_zlib" != "xno"], [
 	AC_MSG_NOTICE([zlib support not requested])
 ])
 
+AC_ARG_WITH([bashcompletiondir],
+	AS_HELP_STRING([--with-bashcompletiondir=DIR], [Bash completions directory]),
+	[],
+	[AS_IF([$($PKG_CONFIG --exists bash-completion)], [
+		with_bashcompletiondir=$($PKG_CONFIG --variable=completionsdir bash-completion)
+	] , [
+		with_bashcompletiondir=${datadir}/bash-completion/completions
+	])])
+AC_SUBST([bashcompletiondir], [$with_bashcompletiondir])
 
 #####################################################################
 # --enable-
@@ -194,7 +203,7 @@ AC_CONFIG_FILES([
 AC_OUTPUT
 AC_MSG_RESULT([
 	$PACKAGE $VERSION
-	======
+	=======
 
 	prefix:			${prefix}
 	sysconfdir:		${sysconfdir}
@@ -202,6 +211,7 @@ AC_MSG_RESULT([
 	rootlibdir:		${rootlibdir}
 	includedir:		${includedir}
 	bindir:			${bindir}
+	Bash completions dir:   ${with_bashcompletiondir}
 
 	compiler:		${CC}
 	cflags:			${with_cflags} ${CFLAGS}
-- 
1.8.3.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] shell-completion: Add initial completion for kmod
  2013-07-30  6:48 [PATCH 1/3] shell-completion: Add initial completion for kmod Lucas De Marchi
  2013-07-30  6:48 ` [PATCH 2/3] shell-completion: Add kmod static-nodes Lucas De Marchi
  2013-07-30  6:48 ` [PATCH 3/3] build: Install bash completion data Lucas De Marchi
@ 2013-07-30 13:17 ` Dave Reisner
  2013-07-30 14:25   ` Lucas De Marchi
  2 siblings, 1 reply; 7+ messages in thread
From: Dave Reisner @ 2013-07-30 13:17 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules

On Tue, Jul 30, 2013 at 03:48:13AM -0300, Lucas De Marchi wrote:
> From: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> Skeleton pulled from udevadm in systemd and adapted to kmod needs.
> ---
>  NEWS                       |  2 +-
>  shell-completion/bash/kmod | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 1 deletion(-)
>  create mode 100644 shell-completion/bash/kmod
> 
> diff --git a/NEWS b/NEWS
> index 1dff366..491146d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -6,7 +6,7 @@ kmod 15
>  
>  - New features:
>  	- kmod static-nodes creates parent directories if given a -o option
> -	- kmod binary doesn't statically link to libkmod
> +	- kmod binary statically link to libkmod
>  
>  kmod 14
>  =======
> diff --git a/shell-completion/bash/kmod b/shell-completion/bash/kmod
> new file mode 100644
> index 0000000..81dbf46
> --- /dev/null
> +++ b/shell-completion/bash/kmod
> @@ -0,0 +1,53 @@
> +# kmod completion                                          -*- shell-script -*-
> +#
> +# This file is part of systemd.
> +#
> +# Copyright 2010 Ran Benita
> +# Copyright (C) 2013  Intel Corporation. All rights reserved.
> +#
> +# systemd is free software; you can redistribute it and/or modify it
> +# under the terms of the GNU Lesser General Public License as published by
> +# the Free Software Foundation; either version 2.1 of the License, or
> +# (at your option) any later version.
> +#
> +# systemd is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public License
> +# along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +
> +__contains_word () {
> +    local word=$1; shift
> +    for w in $*; do [[ $w = $word ]] && return 0; done

Should be "$@", not $*, and "$word", not $word.

> +    return 1
> +}
> +
> +_kmod() {
> +    local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
> +    local verb comps
> +
> +    local -A VERBS=(
> +	[HELP]='help'
> +	[LIST]='list'
> +	[STATIC-NODES]='static-nodes'

It seems you have tabs here instead of spaces. I think you probably want
the top level actions in a single list, not broken out like this.
Subsequent options for the toplevel actions would want to be broken out
in an associative array like you have here.

> +    )
> +
> +    for ((i=0; $i <= $COMP_CWORD; i++)); do
> +        if __contains_word "${COMP_WORDS[i]}" ${VERBS[*]} &&
> +            ! __contains_word "${COMP_WORDS[i-1]}" ${OPTS[ARG]}; then
> +            verb=${COMP_WORDS[i]}
> +            break
> +        fi
> +    done
> +
> +    if [[ -z $verb ]]; then
> +	comps=${VERBS[*]}

Tabs here too.

> +    fi
> +
> +    COMPREPLY=( $(compgen -W '$comps' -- "$cur" ) )
> +    return 0
> +}
> +
> +complete -F _kmod kmod
> -- 
> 1.8.3.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-modules" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] shell-completion: Add initial completion for kmod
  2013-07-30 13:17 ` [PATCH 1/3] shell-completion: Add initial completion for kmod Dave Reisner
@ 2013-07-30 14:25   ` Lucas De Marchi
  2013-07-30 16:54     ` Dave Reisner
  0 siblings, 1 reply; 7+ messages in thread
From: Lucas De Marchi @ 2013-07-30 14:25 UTC (permalink / raw)
  To: Lucas De Marchi, linux-modules

On Tue, Jul 30, 2013 at 10:17 AM, Dave Reisner <d@falconindy.com> wrote:
> On Tue, Jul 30, 2013 at 03:48:13AM -0300, Lucas De Marchi wrote:
>> From: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> Skeleton pulled from udevadm in systemd and adapted to kmod needs.
>> ---
>>  NEWS                       |  2 +-
>>  shell-completion/bash/kmod | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 54 insertions(+), 1 deletion(-)
>>  create mode 100644 shell-completion/bash/kmod
>>
>> diff --git a/NEWS b/NEWS
>> index 1dff366..491146d 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -6,7 +6,7 @@ kmod 15
>>
>>  - New features:
>>       - kmod static-nodes creates parent directories if given a -o option
>> -     - kmod binary doesn't statically link to libkmod
>> +     - kmod binary statically link to libkmod
>>
>>  kmod 14
>>  =======
>> diff --git a/shell-completion/bash/kmod b/shell-completion/bash/kmod
>> new file mode 100644
>> index 0000000..81dbf46
>> --- /dev/null
>> +++ b/shell-completion/bash/kmod
>> @@ -0,0 +1,53 @@
>> +# kmod completion                                          -*- shell-script -*-
>> +#
>> +# This file is part of systemd.
>> +#
>> +# Copyright 2010 Ran Benita
>> +# Copyright (C) 2013  Intel Corporation. All rights reserved.
>> +#
>> +# systemd is free software; you can redistribute it and/or modify it
>> +# under the terms of the GNU Lesser General Public License as published by
>> +# the Free Software Foundation; either version 2.1 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# systemd is distributed in the hope that it will be useful, but
>> +# WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +# General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU Lesser General Public License
>> +# along with systemd; If not, see <http://www.gnu.org/licenses/>.
>> +
>> +__contains_word () {
>> +    local word=$1; shift
>> +    for w in $*; do [[ $w = $word ]] && return 0; done
>
> Should be "$@", not $*, and "$word", not $word.

As said in the commit message, this was taken from systemd. You may
want to change there as well ;-)

>
>> +    return 1
>> +}
>> +
>> +_kmod() {
>> +    local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
>> +    local verb comps
>> +
>> +    local -A VERBS=(
>> +     [HELP]='help'
>> +     [LIST]='list'
>> +     [STATIC-NODES]='static-nodes'
>
> It seems you have tabs here instead of spaces. I think you probably want

It seems like emacs is not passing the experimentation period.

> the top level actions in a single list, not broken out like this.
> Subsequent options for the toplevel actions would want to be broken out
> in an associative array like you have here.

But then how can I take the different actions needed by -f, -o?

Lucas De Marchi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] shell-completion: Add initial completion for kmod
  2013-07-30 14:25   ` Lucas De Marchi
@ 2013-07-30 16:54     ` Dave Reisner
  2013-07-30 17:53       ` Lucas De Marchi
  0 siblings, 1 reply; 7+ messages in thread
From: Dave Reisner @ 2013-07-30 16:54 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Lucas De Marchi, linux-modules

On Tue, Jul 30, 2013 at 11:25:27AM -0300, Lucas De Marchi wrote:
> On Tue, Jul 30, 2013 at 10:17 AM, Dave Reisner <d@falconindy.com> wrote:
> > On Tue, Jul 30, 2013 at 03:48:13AM -0300, Lucas De Marchi wrote:
> >> From: Lucas De Marchi <lucas.demarchi@intel.com>
> >>
> >> Skeleton pulled from udevadm in systemd and adapted to kmod needs.
> >> ---
> >>  NEWS                       |  2 +-
> >>  shell-completion/bash/kmod | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 54 insertions(+), 1 deletion(-)
> >>  create mode 100644 shell-completion/bash/kmod
> >>
> >> diff --git a/NEWS b/NEWS
> >> index 1dff366..491146d 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -6,7 +6,7 @@ kmod 15
> >>
> >>  - New features:
> >>       - kmod static-nodes creates parent directories if given a -o option
> >> -     - kmod binary doesn't statically link to libkmod
> >> +     - kmod binary statically link to libkmod
> >>
> >>  kmod 14
> >>  =======
> >> diff --git a/shell-completion/bash/kmod b/shell-completion/bash/kmod
> >> new file mode 100644
> >> index 0000000..81dbf46
> >> --- /dev/null
> >> +++ b/shell-completion/bash/kmod
> >> @@ -0,0 +1,53 @@
> >> +# kmod completion                                          -*- shell-script -*-
> >> +#
> >> +# This file is part of systemd.
> >> +#
> >> +# Copyright 2010 Ran Benita
> >> +# Copyright (C) 2013  Intel Corporation. All rights reserved.
> >> +#
> >> +# systemd is free software; you can redistribute it and/or modify it
> >> +# under the terms of the GNU Lesser General Public License as published by
> >> +# the Free Software Foundation; either version 2.1 of the License, or
> >> +# (at your option) any later version.
> >> +#
> >> +# systemd is distributed in the hope that it will be useful, but
> >> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> +# General Public License for more details.
> >> +#
> >> +# You should have received a copy of the GNU Lesser General Public License
> >> +# along with systemd; If not, see <http://www.gnu.org/licenses/>.
> >> +
> >> +__contains_word () {
> >> +    local word=$1; shift
> >> +    for w in $*; do [[ $w = $word ]] && return 0; done
> >
> > Should be "$@", not $*, and "$word", not $word.
> 
> As said in the commit message, this was taken from systemd. You may
> want to change there as well ;-)
> 

Yeah, been meaning to do that for a while now.

> >
> >> +    return 1
> >> +}
> >> +
> >> +_kmod() {
> >> +    local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
> >> +    local verb comps
> >> +
> >> +    local -A VERBS=(
> >> +     [HELP]='help'
> >> +     [LIST]='list'
> >> +     [STATIC-NODES]='static-nodes'
> >
> > It seems you have tabs here instead of spaces. I think you probably want
> 
> It seems like emacs is not passing the experimentation period.
> 
> > the top level actions in a single list, not broken out like this.
> > Subsequent options for the toplevel actions would want to be broken out
> > in an associative array like you have here.
> 
> But then how can I take the different actions needed by -f, -o?

Writing completion for a "multi-call" tool like this is painful. I'd
suggest switching on the verb and writing a separate function for each
one. I guess udevadm does something similar to this, but it doesn't mean
I have to agree with it ;) It's a strange mapping, and I don't really
see what it accomplishes.

> Lucas De Marchi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-modules" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] shell-completion: Add initial completion for kmod
  2013-07-30 16:54     ` Dave Reisner
@ 2013-07-30 17:53       ` Lucas De Marchi
  0 siblings, 0 replies; 7+ messages in thread
From: Lucas De Marchi @ 2013-07-30 17:53 UTC (permalink / raw)
  To: Lucas De Marchi, Lucas De Marchi, linux-modules

On Tue, Jul 30, 2013 at 1:54 PM, Dave Reisner <d@falconindy.com> wrote:
> On Tue, Jul 30, 2013 at 11:25:27AM -0300, Lucas De Marchi wrote:
>> On Tue, Jul 30, 2013 at 10:17 AM, Dave Reisner <d@falconindy.com> wrote:
>> > On Tue, Jul 30, 2013 at 03:48:13AM -0300, Lucas De Marchi wrote:
>> >> From: Lucas De Marchi <lucas.demarchi@intel.com>
>> >>
>> >> Skeleton pulled from udevadm in systemd and adapted to kmod needs.
>> >> ---
>> >>  NEWS                       |  2 +-
>> >>  shell-completion/bash/kmod | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>> >>  2 files changed, 54 insertions(+), 1 deletion(-)
>> >>  create mode 100644 shell-completion/bash/kmod
>> >>
>> >> diff --git a/NEWS b/NEWS
>> >> index 1dff366..491146d 100644
>> >> --- a/NEWS
>> >> +++ b/NEWS
>> >> @@ -6,7 +6,7 @@ kmod 15
>> >>
>> >>  - New features:
>> >>       - kmod static-nodes creates parent directories if given a -o option
>> >> -     - kmod binary doesn't statically link to libkmod
>> >> +     - kmod binary statically link to libkmod
>> >>
>> >>  kmod 14
>> >>  =======
>> >> diff --git a/shell-completion/bash/kmod b/shell-completion/bash/kmod
>> >> new file mode 100644
>> >> index 0000000..81dbf46
>> >> --- /dev/null
>> >> +++ b/shell-completion/bash/kmod
>> >> @@ -0,0 +1,53 @@
>> >> +# kmod completion                                          -*- shell-script -*-
>> >> +#
>> >> +# This file is part of systemd.
>> >> +#
>> >> +# Copyright 2010 Ran Benita
>> >> +# Copyright (C) 2013  Intel Corporation. All rights reserved.
>> >> +#
>> >> +# systemd is free software; you can redistribute it and/or modify it
>> >> +# under the terms of the GNU Lesser General Public License as published by
>> >> +# the Free Software Foundation; either version 2.1 of the License, or
>> >> +# (at your option) any later version.
>> >> +#
>> >> +# systemd is distributed in the hope that it will be useful, but
>> >> +# WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> >> +# General Public License for more details.
>> >> +#
>> >> +# You should have received a copy of the GNU Lesser General Public License
>> >> +# along with systemd; If not, see <http://www.gnu.org/licenses/>.
>> >> +
>> >> +__contains_word () {
>> >> +    local word=$1; shift
>> >> +    for w in $*; do [[ $w = $word ]] && return 0; done
>> >
>> > Should be "$@", not $*, and "$word", not $word.
>>
>> As said in the commit message, this was taken from systemd. You may
>> want to change there as well ;-)
>>
>
> Yeah, been meaning to do that for a while now.
>
>> >
>> >> +    return 1
>> >> +}
>> >> +
>> >> +_kmod() {
>> >> +    local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
>> >> +    local verb comps
>> >> +
>> >> +    local -A VERBS=(
>> >> +     [HELP]='help'
>> >> +     [LIST]='list'
>> >> +     [STATIC-NODES]='static-nodes'
>> >
>> > It seems you have tabs here instead of spaces. I think you probably want
>>
>> It seems like emacs is not passing the experimentation period.
>>
>> > the top level actions in a single list, not broken out like this.
>> > Subsequent options for the toplevel actions would want to be broken out
>> > in an associative array like you have here.
>>
>> But then how can I take the different actions needed by -f, -o?
>
> Writing completion for a "multi-call" tool like this is painful. I'd
> suggest switching on the verb and writing a separate function for each
> one. I guess udevadm does something similar to this, but it doesn't mean
> I have to agree with it ;) It's a strange mapping, and I don't really
> see what it accomplishes.

A follow up: like you suggested here and on IRC... each subcommand now
can have its own function and is called with "if declare -F
_kmod_${func} > /dev/null; then _kmod_${func}; fi" in the end of
_kmod function


Thanks, it's now pushed.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-07-30 17:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-30  6:48 [PATCH 1/3] shell-completion: Add initial completion for kmod Lucas De Marchi
2013-07-30  6:48 ` [PATCH 2/3] shell-completion: Add kmod static-nodes Lucas De Marchi
2013-07-30  6:48 ` [PATCH 3/3] build: Install bash completion data Lucas De Marchi
2013-07-30 13:17 ` [PATCH 1/3] shell-completion: Add initial completion for kmod Dave Reisner
2013-07-30 14:25   ` Lucas De Marchi
2013-07-30 16:54     ` Dave Reisner
2013-07-30 17:53       ` Lucas De Marchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.