scop / bash-completion

Programmable completion functions for bash
GNU General Public License v2.0
2.92k stars 380 forks source link

Completion of valid paths involving variables adds backslashes #290

Open inconstante opened 5 years ago

inconstante commented 5 years ago

In _quote_readline_by_ref, unset variables get escaped, whereas set variables don't, for instance:

less /var/log/$@/<TAB>
less /var/log/\$@/

less /var/log/$PWD/<TAB>
less /var/log/$PWD/

Originally reported at https://bugs.debian.org/922657

scop commented 5 years ago

No ideas yet, but we have a history of issues with completions involving variables. In this case I don't think it's a matter of definedness; in your above case it's seemingly related to whether the path including the variable expands to a valid dir. If it does, then the backslashes appear; if it doesn't, they don't. For example:

$ unset foo
$ less /var/log/$foo/
Display all 103 possibilities? (y or n)
$ less /var/log/\$foo/

$foo was unset, so /var/log/$foo/ expanded to /var/log// and offers the 103 files I have there. $foo got escaped. This is consistent with your test case.

$ foo=bar
$ less /var/log/$foo/

No completions offered, /var/log/bar/ does not exist, $foo was not backslashed. Also consistent with your test case.

$ foo=apt
$ less /var/log/$foo/
eipp.log.xz        history.log.3.gz   term.log           term.log.4.gz
history.log        history.log.4.gz   term.log.10.gz     term.log.5.gz
history.log.10.gz  history.log.5.gz   term.log.11.gz     term.log.6.gz
history.log.11.gz  history.log.6.gz   term.log.12.gz     term.log.7.gz
history.log.12.gz  history.log.7.gz   term.log.1.gz      term.log.8.gz
history.log.1.gz   history.log.8.gz   term.log.2.gz      term.log.9.gz
history.log.2.gz   history.log.9.gz   term.log.3.gz      $ less /var/log/\$foo/

Contents of /var/log/apt/ offered as completions, $foo did get escaped, unlike in your test case.

Astara commented 4 years ago

Mar 10, Apr 21 and not fixed? I filed this as a new bug 1) because the above doesn't clearly state it as a bug, 2) because this "issue" isn't labeled as a bug (when it clearly is), 3) because this issue seems to be dead (not moving toward a fix), 4) due to the frequency that people report this problem as a bug in bash on the bug-bash list until the bash-author asks them if they have bash-completion installed as it's been a long outstanding problem with bash-completion and 5) I don't have write access to this bug and can't update things like the title to at least make it clear that it is a bug.

I don't understand. This isn't a bug in bash's native expansion/completion, but is only seen when people install this package. Which file is causing this?

Also, I don't understand why you suggested updating this from 2.5->2.8 when 2.8 doesn't fix the problem. I wasn't experiencing any other problems...

thanks.

scop commented 4 years ago

1, 2, and 5: This project doesn't really use labels. 3: Yes, this project suffers from chronic lack of manpower. Writing replies like this and trying to keep the issue tracker relatively clean from duplicates etc consumes some of that. 4: Yes, as said it's a longstanding issue. And unfortunate.

Our bash_completion file is the cause, and I guess the _variables function. Fixes welcome.

I didn't suggest upgrading to any particular version. Anyway, good to hear you haven't run into any other issues, a lot of them have been fixed nevertheless. There's pretty much only one "supported" version, which is the latest release, and fixes will only appear in git and new releases. Therefore it makes sense to use the newest when reporting bugs or checking if they have been fixed.

You're welcome.

Mark-Joy commented 2 years ago

I just created a fix here.

https://user-images.githubusercontent.com/22109528/141687890-2b511725-23cc-4cb6-a77b-132f7dac5652.mp4

Mark-Joy commented 2 years ago

Complete bash source code including the fix is here You may want to use it with the bellow change in bash_completion version 2.11

diff -uNr bash_completion.orig bash_completion                         --- bash_completion.orig        2021-10-22 22:25:21.000000000 +0700
+++ bash_completion     2021-11-10 10:59:09.204587448 +0700
@@ -570,6 +570,11 @@
     local -a toks
     local reset arg=${1-}

+    if [[ $cur == \'* || $cur == \"* ]]; then
+       # Leave out first character
+       cur="${cur:1}"
+    fi
+
     if [[ $arg == -d ]]; then
         reset=$(shopt -po noglob)
         set -o noglob
@@ -578,8 +583,8 @@
         $reset
         IFS=$'\n'
     else
-        local quoted
-        _quote_readline_by_ref "${cur-}" quoted
+        local quoted="$cur"
+        ##_quote_readline_by_ref "${cur-}" quoted

         # Munge xspec to contain uppercase version too
         # https://lists.gnu.org/archive/html/bug-bash/2010-09/msg00036.html
SamB commented 2 years ago

@Mark-Joy wrote:

Complete bash source code including the fix is here

Wow that's more code than I was hoping. Have you attempted to get this fixed in bash upstream? I realize the copyright assignment is a pain, but perhaps you'll get lucky and they'll fix it a different way.

Either way, it's obviously important to provide a clear description of the problematic behavior (perhaps mostly by reference) rather than just sending them this 200+-line patch to what might be the hairiest part of the shell.

Meanwhile, perhaps I'll go and install zsh on my phone ...