Should I send mail here to get someone's attention?  Or is filing
bugs with [patch] in the name a good way to go?

 I rebased all my recent patches against the current git head, and
attached them, in case anyone else thinks they're interesting.

 I've been going through Ubuntu's
https://bugs.launchpad.net/ubuntu/+source/bash-completion
and closed about 25 bugs that didn't exist anymore, but hadn't been
marked Fix Released.  Or that were invalid.  I fixed some others, or
at least submitted them upstream to Alioth.

 I also wrote patches for apport and dput completions, and reassigned
those bugs to the right packages.  (people don't check if a package
supplies its own completions before filing a bug with bash-completion.)

 I also had some thoughts on how Ubuntu enables completion out of the
box for login shells but not non-login shells.  (tl;dr: that's silly.)
https://bugs.launchpad.net/ubuntu/+source/base-files/+bug/1173728
If Debian is the same way, then my suggestion to use /etc/skel/.bashrc
applies there, too.


 Anyway, here are my patches.  It'd be cool if the useful ones got
applied.  And if someone could explain why the non-useful ones aren't.
:P

 There are still some problems that I haven't really looked into
trying to fix, like:
No completion of filenames inside ` or $() command substitution
https://alioth.debian.org/tracker/index.php?func=detail&aid=314895&group_id=100114&atid=413095


BTS links for my patches:

bugfix for unquoted use of a variable that could contain * and cause
problems.  (on Ubuntu 14.04's version of bash_completion, this fixed
ls /s*[TAB] -> /sbin, even when there were multiple possible results.
git head doesn't have this problem, even with the unquoted use, but
IDK why.)
https://alioth.debian.org/tracker/index.php?func=detail&aid=314891&group_id=100114&atid=413095

Some code cleanup, quoting more stuff that was used unquoted.
Probably just cosmetic.  (patches 2 and 3)
https://alioth.debian.org/tracker/index.php?func=detail&aid=314892&group_id=100114&atid=413095

More restrictive test before an eval, prevented an error in Ubuntu
14.04's bash_completion, but no effect on git head behaviour.
I don't understand this code well enough to be sure this is a good
change. >.<
https://alioth.debian.org/tracker/?func=detail&aid=314667&group_id=100114&atid=413095

_longopt: correctly handle things like grep's --help output line:
  -r, --recursive           like --directories=recurse
 And try to complete on options if the word begins with -, regardless
of what our heuristics say this word should be.  Don't get in the way
of the user.
https://alioth.debian.org/tracker/?func=detail&group_id=100114&aid=314896&atid=413095

upstart support for listing services
https://alioth.debian.org/tracker/?func=detail&group_id=100114&aid=314897&atid=413095


prefetch /etc/bash_completion.d/* into disk cache in the background
https://alioth.debian.org/tracker/index.php?func=detail&aid=314901&group_id=100114&atid=413095


-- 
#define X(x,y) x##y
Peter Cordes ;  e-mail: X(peter@cor , des.ca)

"The gods confound the man who first found out how to distinguish the hours!
 Confound him, too, who in this place set up a sundial, to cut and hack
 my day so wretchedly into small pieces!" -- Plautus, 200 BC
From 03b3f10f01a8e6c45c8a2f32aa981ed12b6c13da Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Sat, 29 Nov 2014 02:11:24 -0400
Subject: [PATCH 1/7] fix _filedir unquoted use of user input.

This fixes the bug where completing a glob turned it into the first match, even when actually ambiguous.
---
 bash_completion | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bash_completion b/bash_completion
index 55c9e48661028cee71301fd244c12d516303d437..f61ec3d578b9f093a3f7c6f28ba39956c48f5a86 100644
--- a/bash_completion
+++ b/bash_completion
@@ -577,7 +577,7 @@ _filedir()
         # Munge xspec to contain uppercase version too
         # http://thread.gmane.org/gmane.comp.shells.bash.bugs/15294/focus=15306
         xspec=${1:+"!*.@($1|${1^^})"}
-        x=$( compgen -f -X "$xspec" -- $quoted ) &&
+        x=$( compgen -f -X "$xspec" -- "$quoted" ) &&
         while read -r tmp; do
             toks+=( "$tmp" )
         done <<< "$x"
@@ -586,7 +586,7 @@ _filedir()
     # If the filter failed to produce anything, try without it if configured to
     [[ -n ${COMP_FILEDIR_FALLBACK:-} && \
         -n "$1" && "$1" != -d && ${#toks[@]} -lt 1 ]] && \
-        x=$( compgen -f -- $quoted ) &&
+        x=$( compgen -f -- "$quoted" ) &&
         while read -r tmp; do
             toks+=( "$tmp" )
         done <<< "$x"
-- 
2.1.3

From 9611de43cb05485c09062451df5a68c2b2620a0a Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Sat, 29 Nov 2014 02:21:32 -0400
Subject: [PATCH 2/7] Quote some unquoted uses variables

Unsure if any of these were needed, maybe they couldn't contain anything they shouldn't.
---
 bash_completion | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/bash_completion b/bash_completion
index f61ec3d578b9f093a3f7c6f28ba39956c48f5a86..628298139fdae8702615c7d3169ac37dd7891cd8 100644
--- a/bash_completion
+++ b/bash_completion
@@ -119,7 +119,7 @@ _have()
 {
     # Completions for system administrator commands are installed as well in
     # case completion is attempted via `sudo command ...'.
-    PATH=$PATH:/usr/sbin:/sbin:/usr/local/sbin type $1 &>/dev/null
+    PATH=$PATH:/usr/sbin:/sbin:/usr/local/sbin type "$1" &>/dev/null
 }
 
 # Backwards compatibility for compat completions that use have().
@@ -128,7 +128,7 @@ _have()
 have()
 {
     unset -v have
-    _have $1 && have=yes
+    _have "$1" && have=yes
 }
 
 # This function checks whether a given readline variable
@@ -626,7 +626,7 @@ _variables()
 {
     if [[ $cur =~ ^(\$\{?)([A-Za-z0-9_]*)$ ]]; then
         [[ $cur == *{* ]] && local suffix=} || local suffix=
-        COMPREPLY+=( $( compgen -P ${BASH_REMATCH[1]} -S "$suffix" -v -- \
+        COMPREPLY+=( $( compgen -P "${BASH_REMATCH[1]}" -S "$suffix" -v -- \
             "${BASH_REMATCH[2]}" ) )
         return 0
     else
@@ -676,7 +676,7 @@ _init_completion()
             e) errx=$OPTARG ;;
             o) outx=$OPTARG ;;
             i) inx=$OPTARG ;;
-            s) split=false ; exclude+== ;;
+            s) split=false ; exclude+='=' ;;
         esac
     done
 
@@ -710,7 +710,7 @@ _init_completion()
                 ;;
         esac
         cur="${cur##$redir}"
-        _filedir $xspec
+        _filedir "$xspec"
         return 1
     fi
 
@@ -1873,10 +1873,10 @@ _install_xspec()
 {
     local xspec=$1 cmd
     shift
-    for cmd in $@; do
+    for cmd in "$@"; do
         _xspecs[$cmd]=$xspec
     done
-    complete -F _filedir_xspec $@
+    complete -F _filedir_xspec "$@"
 }
 # bzcmp, bzdiff, bz*grep, bzless, bzmore intentionally not here, see Debian: #455510
 _install_xspec '!*.?(t)bz?(2)' bunzip2 bzcat pbunzip2 pbzcat lbunzip2 lbzcat
@@ -1978,7 +1978,7 @@ _xfunc()
     set -- "$@"
     local srcfile=$1
     shift
-    declare -F $1 &>/dev/null || {
+    declare -F "$1" &>/dev/null || {
         local compdir=./completions
         [[ $BASH_SOURCE == */* ]] && compdir="${BASH_SOURCE%/*}/completions"
         . "$compdir/$srcfile"
-- 
2.1.3

From e1da9e158c853dfe0f0f9b806684cd314f23fd60 Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Sat, 29 Nov 2014 02:35:43 -0400
Subject: [PATCH 3/7] More / tidier quoting on evals

Not sure what if anything the change in _command_offset fixes, but I assume evaluating the right hand side twice wasn't intended.

 The change in upvars is cosmetic, for readability.  (single quotes to protect stuff from eval is easy to read, but the printf -v $ref %s "$newvalue" idiom is even better).  These couple changes are all that's left from my changes to Ubuntu 14.04's file, since between then and current, many of the evals were converted to the printf idiom.
---
 bash_completion | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bash_completion b/bash_completion
index 628298139fdae8702615c7d3169ac37dd7891cd8..b4da58b64c90428f896cc1afe988e8abf1db17f7 100644
--- a/bash_completion
+++ b/bash_completion
@@ -208,13 +208,13 @@ _upvars()
                     "${FUNCNAME[0]}: \`$1': invalid number specifier" 1>&2
                     return 1; }
                 # Assign array of -aN elements
-                [[ "$2" ]] && unset -v "$2" && eval $2=\(\"\${@:3:${1#-a}}\"\) &&
+                [[ "$2" ]] && unset -v "$2" && eval $2='("${@:3:${1#-a}}")' &&
                 shift $((${1#-a} + 2)) || { echo "bash: ${FUNCNAME[0]}:"\
                     "\`$1${2+ }$2': missing argument(s)" 1>&2; return 1; }
                 ;;
             -v)
                 # Assign single value
-                [[ "$2" ]] && unset -v "$2" && eval $2=\"\$3\" &&
+                [[ "$2" ]] && unset -v "$2" && eval $2='$3' &&
                 shift 3 || { echo "bash: ${FUNCNAME[0]}: $1: missing"\
                 "argument(s)" 1>&2; return 1; }
                 ;;
@@ -1746,7 +1746,7 @@ _command_offset()
             else
                 cspec=${cspec#complete}
                 cspec=${cspec%%$compcmd}
-                COMPREPLY=( $( eval compgen "$cspec" -- '$cur' ) )
+                COMPREPLY=( $( eval compgen "$cspec" -- '"$cur"' ) )
             fi
         elif [[ ${#COMPREPLY[@]} -eq 0 ]]; then
             # XXX will probably never happen as long as completion loader loads
-- 
2.1.3

From e005b41ba69f88945439d11c9755cf3a6ea5307c Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Sat, 29 Nov 2014 02:39:43 -0400
Subject: [PATCH 4/7] _quote_readline_by_ref() only eval if the string starts
 with $', not just $

Fixes errors like echo $(echo f[TAB]
bash: unexpected EOF while looking for matching `)'
bash: syntax error: unexpected end of file

but doesn't actually make tab completion for arguments to commands inside command-substitution constructs work. :(

This change just makes the code match what the comments say it's for; I have no idea when that's needed or if there's a way to get completion of filenames inside $(), other than M-/.
---
 bash_completion | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bash_completion b/bash_completion
index b4da58b64c90428f896cc1afe988e8abf1db17f7..65aea97a41989061cefdeb00e2840dd2cb00ae47 100644
--- a/bash_completion
+++ b/bash_completion
@@ -548,7 +548,7 @@ _quote_readline_by_ref()
     # If result becomes quoted like this: $'string', re-evaluate in order to
     # drop the additional quoting.  See also: http://www.mail-archive.com/
     # bash-completion-devel@lists.alioth.debian.org/msg01942.html
-    [[ ${!2} == \$* ]] && eval $2=${!2}
+    [[ ${!2} == \$\'* ]] && eval $2=${!2}
 } # _quote_readline_by_ref()
 
 
-- 
2.1.3

From b88173ca80fcdac1743549c95bcc0a5c6fc63f0d Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Sun, 30 Nov 2014 11:03:39 -0400
Subject: [PATCH 5/7] _longopt: fix parsing --help output that has -- in the
 description

This fixes parsing of things like grep --help:
  -r, --recursive like --directories=recurse

using \([^-]\|-[^-]\)* instead of .* at the front of the pattern makes the greedy match at the front stop at the first --, rather than getting --directories= from the -r line.

 Also move option completion ahead of the logic that checks previous arg to see if this arg should be limited to a file or directory.  Too smart for its own good in such a naive function, crossed up by things like ls --directory or grep --files-with-matches.

 The sed in the case $prev block doesn't need this, because it puts $prev into the pattern, and it's already presumably a valid option.  It will get the right --option wherever it is in the line containing it.

 Also turns out that bash sorts and uniquifies the results itself, so sort -u isn't needed.

Still not perfect, misses --silent from the help output line:
    -q, --quiet, --silent     suppress all normal output

 Could maybe loop over the --matches in sed, now that we have a sufficiently non-greedy regex to match things in front of --options, but then you'd need a full-blown sed program with pattern and hold space... yuck.  Or maybe use awk?  Or hardcode a pattern that can match up to 3 long options on one line?

 This also breaks on commands with weird --help output, like if they for some reason have --something BEFORE an option name.  You could start to work around that, with another group like --[^-A-Za-z0-9] to match a -- that isn't at the start of an option, but that's just gratuitously unreadable.  Do that for more robustness if anyone ever turns it into a sed program that loops over --option matches on a single line.
---
 bash_completion | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/bash_completion b/bash_completion
index 65aea97a41989061cefdeb00e2840dd2cb00ae47..f326e6dc2b32a634e7cd39621650c54fbdc64bc3 100644
--- a/bash_completion
+++ b/bash_completion
@@ -1777,6 +1777,20 @@ _longopt()
     local cur prev words cword split
     _init_completion -s || return
 
+    # Check for options first: some programs have options like
+    # --directory=recursive that don't take directory args
+    # It's more likely the user knows what they're doing,
+    # for this naive --help parsing function.
+    if [[ "$cur" == -* ]]; then
+        COMPREPLY=( $( compgen -W "$( LC_ALL=C $1 --help 2>&1 | \
+	    sed -ne 's/\([^-]\|-[^-]\)*\(--[-A-Za-z0-9]\{1,\}=\{0,1\}\).*/\2/p' )" \
+            -- "$cur" ) )
+	    # initial part of that regex matches only up to before the first --,
+	    # to avoid tripping on " -r, --recursive   like --directory=recursive" in grep --help, for example.
+        [[ $COMPREPLY == *= ]] && compopt -o nospace
+        return 0
+    fi
+
     case "${prev,,}" in
         --help|--usage|--version)
             return 0
@@ -1807,12 +1821,7 @@ _longopt()
 
     $split && return 0
 
-    if [[ "$cur" == -* ]]; then
-        COMPREPLY=( $( compgen -W "$( LC_ALL=C $1 --help 2>&1 | \
-            sed -ne 's/.*\(--[-A-Za-z0-9]\{1,\}=\{0,1\}\).*/\1/p' | sort -u )" \
-            -- "$cur" ) )
-        [[ $COMPREPLY == *= ]] && compopt -o nospace
-    elif [[ "$1" == @(mk|rm)dir ]]; then
+    if [[ "$1" == @(mk|rm)dir ]]; then
         _filedir -d
     else
         _filedir
-- 
2.1.3

From 969b6445ec740736608c0db4e4e98edac014b222 Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Sun, 30 Nov 2014 09:41:57 -0400
Subject: [PATCH 6/7] upstart support for service completion

initctl list works for unprivileged users.

Wasn't sure what file to check to detect that upstart was present, but /sbin should always be mounted, and upstart itself provides /sbin/upstart-dbus-bridge, and it's not a conffile in /etc that someone could move if they wanted to on their local system.  And it's absolutely not going to have a name conflict with anything from another package.  :)

 I think it's important to check that the system is using an upstart init, so you don't run initctl when completing in a root shell on another kind of system, and maybe do something like generating system log messages.
---
 bash_completion | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/bash_completion b/bash_completion
index f326e6dc2b32a634e7cd39621650c54fbdc64bc3..bf00bf1aef8b3cb4656a807c039f8ea20e1b0a57 100644
--- a/bash_completion
+++ b/bash_completion
@@ -1137,6 +1137,10 @@ _services()
     COMPREPLY+=( $( systemctl list-units --full --all 2>/dev/null | \
         awk '$1 ~ /\.service$/ { sub("\\.service$", "", $1); print $1 }' ) )
 
+    if [[ -x /sbin/upstart-dbus-bridge ]]; then
+        COMPREPLY+=( $( initctl list 2>/dev/null | cut -d' ' -f1 ) )
+    fi
+
     COMPREPLY=( $( compgen -W '${COMPREPLY[@]#${sysvdirs[0]}/}' -- "$cur" ) )
 }
 
-- 
2.1.3

From 4987195cc7b59ae62322906eaa7b965803e06c07 Mon Sep 17 00:00:00 2001
From: Peter Cordes <pe...@cordes.ca>
Date: Tue, 2 Dec 2014 10:08:31 -0400
Subject: [PATCH 7/7] speed up loading the compat dir with disk prefetch

Hard drives like to see reads coming as far ahead as possible,
and ideally a queue of reads to service in whatever order is fastest.

 So fork off a   cat * > /dev/null  in the background to pre-read
all the files we're going to access soon, so the OS has them cached.

tail(1) might spend less CPU copying stuff around in RAM (it would make
fewer system calls writing /dev/null), but POSIX tail only takes one arg.

 I'm seeing a factor of 2 speedup for this change, with cold disk cache.
On linux, echo 3 | sudo tee /proc/sys/vm/drop_caches > /dev/null
And no slowdown with hot caches.  (dual core CPU)


 Other changes:
Had to moved the necessary stuff up near the top of the file.
Was able to greatly simplify the loop over BASH_COMPLETION_COMPAT_DIR by
using the glob in the first place, instead of ls and then filtering.

 Took out the check for [[ -r $i ]] before sourcing.  If you have files in
/etc/bash_completion.d that aren't readable, you might not even notice if
bash_completion silently ignores them.  It's not like anything else uses
the directory, so don't be too quiet when there is a problem.

 I've even seen packages put completions in subdirectories (e.g. unison)
Could change from -f to -e to get warnings for that.  A package could
legitimately have a helper function or something in a subdir, though.
---
 bash_completion | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/bash_completion b/bash_completion
index bf00bf1aef8b3cb4656a807c039f8ea20e1b0a57..1224284a5e29ef647ee060a70c7d29ec87417a25 100644
--- a/bash_completion
+++ b/bash_completion
@@ -47,9 +47,38 @@ readonly BASH_COMPLETION_COMPAT_DIR
 #
 _blacklist_glob='@(acroread.sh)'
 
+# Glob for matching various backup files.
+#
+_backup_glob='@(#*#|*@(~|.@(bak|orig|rej|swp|dpkg*|rpm@(orig|new|save))))'
+
 # Turn on extended globbing and programmable completion
 shopt -s extglob progcomp
 
+# source (or prefetch from disk) compat completion directory definitions
+_load_compat_dir()
+{
+    [[ -d $BASH_COMPLETION_COMPAT_DIR ]] || return
+    local i glob="$BASH_COMPLETION_COMPAT_DIR/!($_backup_glob|Makefile*|$_blacklist_glob)"
+
+    if [[ $1 == prefetch ]]; then
+        if [[ ! $BASH_COMPLETION_DISABLE_PREFETCH ]]; then
+            # start loading from disk in the background
+            cat $glob &>/dev/null </dev/null &
+            disown $! 	# don't pollute job control.
+        fi
+    else
+        for i in $glob; do
+            # If there are unreadable files, user probably wants to know,
+            # so don't check -r
+            [[ -f $i ]] && . "$i"
+        done
+    fi
+}
+# called again near the end of this file, and then unset
+_load_compat_dir prefetch
+
+
+
 # A lot of the following one-liners were taken directly from the
 # completion examples provided with the bash 2.04 source distribution
 
@@ -1105,10 +1134,6 @@ _gids()
     fi
 }
 
-# Glob for matching various backup files.
-#
-_backup_glob='@(#*#|*@(~|.@(bak|orig|rej|swp|dpkg*|rpm@(orig|new|save))))'
-
 # Complete on xinetd services
 #
 _xinetd_services()
@@ -1999,16 +2024,8 @@ _xfunc()
     "$@"
 }
 
-# source compat completion directory definitions
-if [[ -d $BASH_COMPLETION_COMPAT_DIR && -r $BASH_COMPLETION_COMPAT_DIR && \
-    -x $BASH_COMPLETION_COMPAT_DIR ]]; then
-    for i in $(LC_ALL=C command ls "$BASH_COMPLETION_COMPAT_DIR"); do
-        i=$BASH_COMPLETION_COMPAT_DIR/$i
-        [[ ${i##*/} != @($_backup_glob|Makefile*|$_blacklist_glob) \
-            && -f $i && -r $i ]] && . "$i"
-    done
-fi
-unset i _blacklist_glob
+_load_compat_dir source
+unset _blacklist_glob _load_compat_dir
 
 # source user completion file
 [[ ${BASH_SOURCE[0]} != ~/.bash_completion && -r ~/.bash_completion ]] \
-- 
2.1.3

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Bash-completion-devel mailing list
Bash-completion-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/bash-completion-devel

Reply via email to