bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: shell-expand-line drops quotation marks [FIXED]


From: Dabrien 'Dabe' Murphy
Subject: Re: shell-expand-line drops quotation marks [FIXED]
Date: Thu, 03 Nov 2016 19:14:58 -0400
User-agent: Postbox 3.0.11 (Macintosh/20140602)

On 11/3/16, 4:21 PM, Chet Ramey wrote:
Quote Removal makes sense during command EXECUTION (since you wouldn't want
your quotes passed in with the arguments) but it doesn't make sense during
(readline) EDITING, IMHO...

OK. So let's talk about a mechanism to provide alternate behavior.  The
conventional way to do that in the context of key bindings is to modify
behavior based on whether or not the user supplies a numeric argument
(e.g., M-1M-C-e).

As if Esc + Control-E wasn't already enough of a contortion!  «grin»

I've actually bound `shell-expand-line` to ^X+Tab — sort of an über expansion, if you will...  I guess one could always add Yet Another level of indirection:

prompt% bind '"\C-x\C-i":"\e1\e\C-e"'   # Ow, my brain!


At the very least, I would expect `shell-expand-line` to be more or less
idempotent; expanding the line multiple times shouldn't change the behavior
of the command that actually gets executed:

This is not a reasonable expection.  Think of it as running a command
through `eval' several times.

prompt% echo $'\007'
<BEL>

prompt% eval eval eval eval eval echo $'\007'
<BEL>

(Okay, so I know you were thinking more along the lines of `eval $(eval $(eval echo echo echo 'Hi There!') )` but I don't think anybody would expect that to be repeatable...)

I will have to think a little bit more, however, about what I mean when I do:

prompt% foo="one two"   # two words
prompt% bar='$foo'      # bare reference
prompt% baz='"$foo"'    # double-quoted reference
prompt% bad=\'\$foo\'   # single-quoted reference

Currently I don't like *EITHER* scenario:

old% echo $bad       [<M-C-e>]
old% echo '$foo'     [<M-C-e>]
old% echo $foo       [<M-C-e>]
old% echo one two

new% echo $bad       [<M-C-e>]
new% echo '$foo'     [<M-C-e>]
new% echo 'one two'

In my mind, '$foo' would be unexpandable...  Ditto:

prompt% echo \$foo   # Should not change anything...

    prompt% alias ls="ls -F"
    prompt% ls [<M-C-e>]
    prompt% ls -F [<M-C-e>]
    prompt% ls -F -F [<M-C-e>]
    prompt% ls -F -F -F

This is actually the correct behavior.  `ls' has an alias, and that alias
is expanded.

I don't disagree that that's what it's doing, but it seems like it could be ensmartened, somehow, to recognize when an alias would expand to itself...

I'm also on the fence about whether I would prefer:

prompt% alias zz=yy
prompt% alias yy=xx
prompt% alias xx=ls

prompt% zz [<M-C-e>]
prompt% yy

or:

prompt% zz [<M-C-e>]
prompt% ls

THAT'S the kind of thing I'd expect a numeric prefix argument to control: whether to recurse or not...  And the logic already exists to prevent the following circular definition from getting stuck in an infinite loop; I would imagine it could be reused to short-circuit the expansion, as well:

prompt% alias xx=yy
prompt% alias yy=zz
prompt% alias zz=xx

prompt% xx
-bash: xx: command not found

Nice catch.  This is pretty close, and gets you most of the way you want
to go. You'd also like a way to inhibit process and command substitution
during shell-expand-line and allow those to be performed as part of
execution to avoid side effects.

Process substitution, yeah, I can't actually imagine it ever being useful to actually SEE the "/dev/fd/##" — any my fix actually inhibits it already...  Though after you mentioned it, I was actually kind of surprised to see the old behavior even worked at all:

old% cat <(echo Hi There) [<M-C-e>]
old% cat /dev/fd/63       [<Enter>]
Hi There

COMMAND substitution, however...  I could see that being useful:

prompt% echo server-$(date +%Y%m%d).log  [<M-C-e>]
prompt% echo server-20161103.log

Or here's a more practical example:

prompt% vi $(grep -ri -l old_text lib/)
prompt% git add !*    # WHOOPS! "old_text" doesn't match anymore

prompt% vi $(grep -ri -l old_text lib/) [<M-C-e>]
prompt% vi lib/Module/foo.c lib/Module/foo.h lib/main.c
prompt% gid add !*    # OKAY


As an aside, I did not expect this:

prompt% echo server-$(date +%Y%m%d).log
server-20161103.log

prompt% echo !* [<M-C-e>]
prompt% echo server-$ ( date +%Y%m%d ) .log
-bash: syntax error near unexpected token `('

But that's not actually related to shell-expand-line, itself:

prompt% echo server-$(date +%Y%m%d).log
server-20161103.log

prompt% echo !*
-bash: syntax error near unexpected token `('

### Quotes, however, seem to make things work again...

prompt% echo "server-$(date +%Y%m%d).log"

server-20161103.log

prompt% echo !*
server-20161103.log

prompt% echo !* [<M-C-e>]
prompt% echo "server-20161103.log"   # OKAY

The old behavior will remain the default for now -- it's been this way for
about 25 years, after all -- but the inhibiting-quote-removal behavior
(and probably command and process substitution as well) will be available
if the user supplies a numeric argument.

My vote would still be to provide a way to make quote preservation the default — e.g., a "quote-shell-expand-line" readline variable:
diff --git a/bashline.c b/bashline.c
index 759e1c5..f5b59e6 100644
--- a/bashline.c
+++ b/bashline.c
@@ -315,6 +315,9 @@ static int dabbrev_expand_active = 0;
 #define COMPLETE_BSQUOTE 3
 static int completion_quoting_style = COMPLETE_BSQUOTE;

+/* When non-zero, preserve quotes during 'shell-expand-line' */
+int _rl_quote_shell_expand_line = 0;
+
 /* Flag values for the final argument to bash_default_completion */
 #define DEFCOMP_CMDPOS         1

@@ -2734,7 +2737,7 @@ shell_expand_line (count, ignore)
       /* If there is variable expansion to perform, do that as a separate
         operation to be undone. */
       new_line = savestring (rl_line_buffer);
-      expanded_string = expand_string (new_line, 0);
+      expanded_string = expand_string (new_line, _rl_quote_shell_expand_line);
       FREE (new_line);
       if (expanded_string == 0)
        {
diff --git a/lib/readline/bind.c b/lib/readline/bind.c
index f1098c4..dbe7d5f 100644
--- a/lib/readline/bind.c
+++ b/lib/readline/bind.c
@@ -1571,6 +1571,7 @@ static const struct {
   { "page-completions",                &_rl_page_completions,          0 },
   { "prefer-visible-bell",     &_rl_prefer_visible_bell,       V_SPECIAL },
   { "print-completions-horizontally", &_rl_print_completions_horizontally, 0 },
+  { "quote-shell-expand-line",  &_rl_quote_shell_expand_line,   0 },
   { "revert-all-at-newline",   &_rl_revert_all_at_newline,     0 },
   { "show-all-if-ambiguous",   &_rl_complete_show_all,         0 },
   { "show-all-if-unmodified",  &_rl_complete_show_unmodified,  0 },
diff --git a/lib/readline/doc/rluser.texi b/lib/readline/doc/rluser.texi
index 4c094c8..4508a0a 100644
--- a/lib/readline/doc/rluser.texi
+++ b/lib/readline/doc/rluser.texi
@@ -686,6 +686,11 @@ If set to @samp{on}, Readline will display completions with matches
 sorted horizontally in alphabetical order, rather than down the screen.
 The default is @samp{off}.

address@hidden quote-shell-expand-line
address@hidden quote-shell-expand-line
+If set to @samp{on}, Readline will preserve quoting when performing
address@hidden  The default [sadly] is @samp{off}.
+
 @item revert-all-at-newline
 @vindex revert-all-at-newline
 If set to @samp{on}, Readline will undo all changes to history lines
diff --git a/lib/readline/rlprivate.h b/lib/readline/rlprivate.h
index fc3856a..04eeab7 100644
--- a/lib/readline/rlprivate.h
+++ b/lib/readline/rlprivate.h
@@ -434,6 +434,9 @@ extern int _rl_vi_domove_motion_cleanup PARAMS((int, _rl_vimotion_cxt *));
  * Undocumented private variables                                       *
  *************************************************************************/

+/* bashline.c */
+extern int _rl_quote_shell_expand_line;
+
 /* bind.c */
 extern const char * const _rl_possible_control_prefixes[];
 extern const char * const _rl_possible_meta_prefixes[];


Then the numeric argument could simply invert this behavior...  OR be used for some other functionality — like recursion, above.  :-)

It's true: globbing is not one of the shell word expansions that this
function performs, or has ever performed.  There's a separate key binding
that does this.

Part of me thinks that variable expansion should have its own binding — like `shell-expand-line` without the history and alias expansion before it.

Then you could just make shell-expand-line a macro that calls `history-expand-line`, `alias-expand-line`, and `variable-expand-line` (and then maybe a new "glob-expand-line" that operates on the whole line, not just the current word.)


PS — Even though it probably sounds like I'm an opinionated, demanding a**, I DO want to say how thankful I am for all your hard work, Chet!  I know it's extremely hard to strike the right balance between adding new features and not breaking existing functionality, and I'm eternally grateful for your tireless effort!

(So at least I'm a GRATEFUL a**...  «heh»)

--
:- Dabe

reply via email to

[Prev in Thread] Current Thread [Next in Thread]