Merge lp:~jjo/bash-completion/fix-bash43-quote_readline_by_ref-tilde-and-double_escaping into lp:ubuntu/trusty/bash-completion

[JuanJo Ciarlante]

[jjo, r=] fix _quote_readline_by_ref to:
- avoid escaping 1st '~' (lp: #1288314)
- avoid quoting if empty, else expansion without args only shows dirs (lp: #1288031)
- replace double escaping to single (eg for completing file/paths with spaces)

[lp:~jjo/bash-completion/fix-bash43-quote_readline_by_ref-tilde-and-double_escaping]

47. By JuanJo Ciarlante on 2014-03-11

fix also lp# 1288031: avoid quoting if empty

48. By JuanJo Ciarlante on 2014-03-11

use string replacing instead of eval to replace double-escaping by single-escaping

[Martin Pitt (pitti)]

Hello JuanJo,

thanks for these fixes! However, the debian/patches/words_bad_array_subscript.patch bit collides with a different fix from Jan Gerber (bug 1289597), so this needs to be dropped.

The other changes need to be done as proper debian/patches/, too (and separated by issue if possible). Did you author them? If yes, can you please send them to upstream? If no, where did you get them from? We need this to track the patch and get it into Debian and Upstream, as it's certainly not Ubuntu specific.

Please set back the status (at the top of the page) to "needs review" when this is sorted out.

Thank you!

review: Needs Fixing

[JuanJo Ciarlante (jjo)]

Hi Martin,

> Hello JuanJo,
>
> thanks for these fixes! However, the
> debian/patches/words_bad_array_subscript.patch bit collides with a different
> fix from Jan Gerber (bug 1289597), so this needs to be dropped.

Indeed, likely a mishandling on my side :parent branch and :push
branches.

>
> The other changes need to be done as proper debian/patches/, too (and
> separated by issue if possible). Did you author them? If yes, can you please
> send them to upstream? If no, where did you get them from? We need this to
> track the patch and get it into Debian and Upstream, as it's certainly not
> Ubuntu specific.

I authored them indeed. Besides fixing ~3 different issues, the lines are
so close that would make patches to have a strict order/dependency and
traces from each (thus kinda defeating a 1-patch-per-issue approach somehow).

Question (as I'm a total newbie regarding ubuntu/debian/upstream workflows):
it's not necessary to wait upstream to have them applied right ?
- thinking on trusty release happening in ~1month, and an army of thousand
cmdliners hating it because of its broken completion :)

>
> Please set back the status (at the top of the page) to "needs review" when
> this is sorted out.

Thanks, will do.

>
> Thank you!

Cheers!,
--JuanJo

[JuanJo Ciarlante (jjo)]

Apologies for changing the new MP (instead of this one), found cleaner to re-start it from scratch.
Quilted the changes as debian/patches/, please review:
   https://code.launchpad.net/~jjo/bash-completion/quote_readline_by_ref_fixes/+merge/210930

Cheers,
--JuanJo

[Martin Pitt (pitti)]

Thanks! The new MP was merged, closing this one.

review: Disapprove

Unmerged revisions

48. By JuanJo Ciarlante on 2014-03-11

use string replacing instead of eval to replace double-escaping by single-escaping

47. By JuanJo Ciarlante on 2014-03-11

fix also lp# 1288031: avoid quoting if empty

46. By JuanJo Ciarlante on 2014-03-11

[jjo, r=] fix _quote_readline_by_ref to avoid escaping 1st '~', and re-eval to fix double escaping

45. By j on 2014-03-09

* debian/patches/words_bad_array_subscript.patch
  - Fix bash: words: bad array subscript (lp: #1289597)

=== modified file 'bash_completion'
--- bash_completion	2014-03-09 17:38:14 +0000
+++ bash_completion	2014-03-11 16:50:27 +0000
@@ -536,13 +536,23 @@
 # @param $2  Name of variable to return result to
 _quote_readline_by_ref()
 {
-    if [[ $1 == \'* ]]; then
+    if [ -z "$1" ]; then
+        # avoid quoting if empty
+        printf -v $2 %s "$1"
+    elif [[ $1 == \'* ]]; then
         # Leave out first character
         printf -v $2 %s "${1:1}"
+    elif [[ $1 == ~* ]]; then
+        # avoid escaping first ~
+        printf -v $2 ~%q "${1:1}"
     else
         printf -v $2 %q "$1"
     fi
 
+    # Replace double escaping ( \\ ) by single ( \ )
+    # This happens always when argument is already escaped at cmdline,
+    # and passed to this function as e.g.: file\ with\ spaces
+    [[ ${!2} == *\\* ]] && printf -v $2 %s "${1//\\\\/\\}"
     # If result becomes quoted like this: $'string', re-evaluate in order to
     # drop the additional quoting.  See also: http://www.mail-archive.com/
     # [e-mail]/msg01942.html
 
=== added file 'debian/patches/words_bad_array_subscript.patch'
--- debian/patches/words_bad_array_subscript.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/words_bad_array_subscript.patch	2014-03-11 16:50:27 +0000
@@ -0,0 +1,13 @@
+diff --git a/bash_completion b/bash_completion
+index 66d36b1..ce469d0 100644
+--- a/bash_completion
++++ b/bash_completion
+@@ -707,7 +707,7 @@ _init_completion()
+         fi
+     done
+ 
+-    [[ $cword -eq 0 ]] && return 1
++    [[ $cword -le 0 ]] && return 1
+     prev=${words[cword-1]}
+ 
+     [[ ${split-} ]] && _split_longopt && split=true
 
=== renamed file 'debian/patches/words_bad_array_subscript.patch' => 'debian/patches/words_bad_array_subscript.patch.moved'