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

Proposed by JuanJo Ciarlante on 2014-03-11
Status: Rejected
Rejected by: Martin Pitt on 2014-03-14
Proposed branch: lp:~jjo/bash-completion/fix-bash43-quote_readline_by_ref-tilde-and-double_escaping
Merge into: lp:ubuntu/trusty/bash-completion
Diff against target: 48 lines (+24/-1) (has conflicts)
2 files modified
bash_completion (+11/-1)
debian/patches/words_bad_array_subscript.patch (+13/-0)
Conflict adding file debian/patches/words_bad_array_subscript.patch.  Moved existing file to debian/patches/words_bad_array_subscript.patch.moved.
To merge this branch: bzr merge lp:~jjo/bash-completion/fix-bash43-quote_readline_by_ref-tilde-and-double_escaping
Reviewer Review Type Date Requested Status
Martin Pitt 2014-03-11 Disapprove on 2014-03-14
Review via email: mp+210421@code.launchpad.net

Description of the change

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

To post a comment you must log in.
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) wrote :

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) wrote :

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) wrote :

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) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bash_completion'
2--- bash_completion 2014-03-09 17:38:14 +0000
3+++ bash_completion 2014-03-11 16:50:27 +0000
4@@ -536,13 +536,23 @@
5 # @param $2 Name of variable to return result to
6 _quote_readline_by_ref()
7 {
8- if [[ $1 == \'* ]]; then
9+ if [ -z "$1" ]; then
10+ # avoid quoting if empty
11+ printf -v $2 %s "$1"
12+ elif [[ $1 == \'* ]]; then
13 # Leave out first character
14 printf -v $2 %s "${1:1}"
15+ elif [[ $1 == ~* ]]; then
16+ # avoid escaping first ~
17+ printf -v $2 ~%q "${1:1}"
18 else
19 printf -v $2 %q "$1"
20 fi
21
22+ # Replace double escaping ( \\ ) by single ( \ )
23+ # This happens always when argument is already escaped at cmdline,
24+ # and passed to this function as e.g.: file\ with\ spaces
25+ [[ ${!2} == *\\* ]] && printf -v $2 %s "${1//\\\\/\\}"
26 # If result becomes quoted like this: $'string', re-evaluate in order to
27 # drop the additional quoting. See also: http://www.mail-archive.com/
28 # bash-completion-devel@lists.alioth.debian.org/msg01942.html
29
30=== added file 'debian/patches/words_bad_array_subscript.patch'
31--- debian/patches/words_bad_array_subscript.patch 1970-01-01 00:00:00 +0000
32+++ debian/patches/words_bad_array_subscript.patch 2014-03-11 16:50:27 +0000
33@@ -0,0 +1,13 @@
34+diff --git a/bash_completion b/bash_completion
35+index 66d36b1..ce469d0 100644
36+--- a/bash_completion
37++++ b/bash_completion
38+@@ -707,7 +707,7 @@ _init_completion()
39+ fi
40+ done
41+
42+- [[ $cword -eq 0 ]] && return 1
43++ [[ $cword -le 0 ]] && return 1
44+ prev=${words[cword-1]}
45+
46+ [[ ${split-} ]] && _split_longopt && split=true
47
48=== renamed file 'debian/patches/words_bad_array_subscript.patch' => 'debian/patches/words_bad_array_subscript.patch.moved'

Subscribers

People subscribed via source and target branches