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
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