[Bash-completion-devel] [PATCH] _command_offset: Restore compopts used by called command.

[Igor Murzov]

This fixes completions that rely on their compopts, most notably mount(8).
Fixes bash-completion bug #313183.

I've tested this patch and it passes tests and I haven't seen any 
regression. But, it is likely that the testsuite is not complete, so I 
can't be sure, that nothing is broke. For example I don't understand 
what the purpose of the code titled by following comment:
  # remove any \: generated by a command that doesn't
  # default to filenames or dirnames

So, please review, test and give feedback.
---
 bash_completion                |   24 +++++++---------
 test/lib/completions/mount.exp |    9 +++++-
 test/lib/completions/sudo.exp  |   59 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/bash_completion b/bash_completion
index bb91f68..322878f 100644
--- a/bash_completion
+++ b/bash_completion
@@ -1542,8 +1542,7 @@ _command()
 
 # A meta-command completion function for commands like sudo(8), which need to
 # first complete on a command, then complete according to that command's own
-# completion definition - currently not quite foolproof (e.g. mount and umount
-# don't work properly), but still quite useful.
+# completion definition.
 #
 _command_offset()
 {
@@ -1605,18 +1604,17 @@ _command_offset()
                     $func $cmd "${COMP_WORDS[${#COMP_WORDS[@]}-1]}"
                 fi
 
-                # remove any \: generated by a command that doesn't
-                # default to filenames or dirnames (e.g. sudo chown)
-                # FIXME: I'm pretty sure this does not work!
-                if [ "${cspec#*-o }" != "$cspec" ]; then
-                    cspec=${cspec#*-o }
-                    cspec=${cspec%% *}
-                    if [[ "$cspec" != @(dir|file)names ]]; then
-                        COMPREPLY=("${COMPREPLY[@]//\\\\:/:}")
-                    else
-                        compopt -o filenames
+                # restore initial compopts
+                local opt t
+                while true; do
+                    t=${cspec#*-o }
+                    if [ "$t" == "$cspec" ]; then
+                        break
                     fi
-                fi
+                    opt=${t%% *}
+                    compopt -o $opt
+                    cspec=${t#$opt}
+                done
             else
                 cspec=${cspec#complete}
                 cspec=${cspec%%$compcmd}
diff --git a/test/lib/completions/mount.exp b/test/lib/completions/mount.exp
index c99d605..1298f36 100644
--- a/test/lib/completions/mount.exp
+++ b/test/lib/completions/mount.exp
@@ -30,7 +30,7 @@ proc setup {} {
 
 proc teardown {} {
     teardown_dummy_mnt
-    assert_env_unmodified
+    assert_env_unmodified {/OLDPWD/d}
 }
 
 
@@ -49,6 +49,13 @@ assert_complete_any "mount -t "
 sync_after_int
 
 
+set test "mount /dev/sda1 def should complete directory name"
+assert_complete_dir "default/" "mount /dev/sda1 def" $::srcdir/fixtures/shared $test -nospace
+
+
+sync_after_int
+
+
 set test "Check completing nfs mounts"
 set expected [list /test/path /test/path2 /second/path]
 set cmd "mount mocksrv:/"
diff --git a/test/lib/completions/sudo.exp b/test/lib/completions/sudo.exp
index d6bbbbb..6108ddc 100644
--- a/test/lib/completions/sudo.exp
+++ b/test/lib/completions/sudo.exp
@@ -24,4 +24,63 @@ assert_complete_dir fixtures/ "sudo sh fix" $::srcdir "" -nospace
 sync_after_int
 
 
+# test that `mount` and `sudo mount` behave the same way
+set test "sudo mount /dev/sda1 def should complete directory name"
+assert_complete_dir "default/" "sudo mount /dev/sda1 def" $::srcdir/fixtures/shared $test -nospace
+
+
+sync_after_int
+
+
+# Find user/group suitable for testing.
+set failed_find_unique_completion 0
+foreach ug {user group} {
+    # compgen -A is used because it's a bash builtin and available everywhere.
+    # The || true part prevents exec from throwing an exception if nothing is
+    # found (very very unlikely).
+    set list [split [exec bash -c "compgen -A $ug || true"] "\n"]
+    if {![find_unique_completion_pair $list part$ug full$ug]} {
+        untested "Not running complex chown tests; no suitable test $ug found."
+        set failed_find_unique_completion 1
+    }
+}
+
+# These tests require an unique completion.
+if {!$failed_find_unique_completion} {
+    assert_complete $fulluser "sudo chown $partuser"
+    sync_after_int
+
+    assert_complete $fulluser:$fullgroup "sudo chown $fulluser:$partgroup"
+    sync_after_int
+
+    assert_complete "dot.user:$fullgroup" "sudo chown dot.user:$partgroup"
+    sync_after_int
+
+    foreach prefix {
+        "funky\\ user:" "funky.user:" "funky\\.user:" "fu\\ nky.user:"
+        "f\\ o\\ o\\.\\bar:" "foo\\_b\\ a\\.r\\ :"
+    } {
+        set test "Check preserve special chars in $prefix$partgroup<TAB>"
+        #assert_complete_into "chown $prefix$partgroup" "chown $prefix$fullgroup " $test
+        assert_complete $prefix$fullgroup "sudo chown $prefix$partgroup" $test
+        sync_after_int
+    }
+
+    # Check that we give up in degenerate cases instead of spewing various junk.
+
+    assert_no_complete "sudo chown $fulluser\\\\:$partgroup"
+    sync_after_int
+
+    assert_no_complete "sudo chown $fulluser\\\\\\:$partgroup"
+    sync_after_int
+
+    assert_no_complete "sudo chown $fulluser\\\\\\\\:$partgroup"
+    sync_after_int
+
+    # Colons in user/groupnames are not usually allowed.
+    assert_no_complete "sudo chown foo:bar:$partgroup"
+    sync_after_int
+}
+
+
 teardown
-- 
1.7.4.4