zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] edit-command-line breaks arguments with spaces
@ 2017-10-13 23:05 Marco Hinz
  2017-10-14  0:07 ` Daniel Shahaf
  0 siblings, 1 reply; 11+ messages in thread
From: Marco Hinz @ 2017-10-13 23:05 UTC (permalink / raw)
  To: zsh-workers

In Functions/Zle/edit-command-line are these two lines:

  local editor=${${VISUAL:-${EDITOR:-vi}}}
  ${=editor} -c "normal! ${byteoffset}go" -- $1;;

In the second line $editor gets word-split. This is a problem in cases like:

  VISUAL='vim -c "set bufhidden=wipe"'

The word splitting splits the argument to -c in two pieces, "set, and bufhidden=wipe".

Here a small script that shows the difference and my suggestion of using eval:

  https://gist.github.com/mhinz/ec6f0363ac083ab80613148d39b28121

Marco

diff --git i/Functions/Zle/edit-command-line w/Functions/Zle/edit-command-line
index 353f2609a..5c82e823a 100644
--- i/Functions/Zle/edit-command-line
+++ w/Functions/Zle/edit-command-line
@@ -22 +22 @@
-      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
+      eval "${editor[@]} -c 'normal! ${byteoffset}go' -- $1";;
@@ -25,2 +25,2 @@
-      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
-    (*) ${=editor} $1;;
+      eval "${editor[@]} +${#lines}:$((${#lines[-1]} + 1)) $1";;
+    (*) eval "${editor[@]} $1";;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-13 23:05 [PATCH] edit-command-line breaks arguments with spaces Marco Hinz
@ 2017-10-14  0:07 ` Daniel Shahaf
  2017-10-14  0:45   ` Marco Hinz
  2017-10-14  0:55   ` Bart Schaefer
  0 siblings, 2 replies; 11+ messages in thread
From: Daniel Shahaf @ 2017-10-14  0:07 UTC (permalink / raw)
  To: Marco Hinz, zsh-workers

Marco Hinz wrote on Sat, 14 Oct 2017 01:05 +0200:
> diff --git i/Functions/Zle/edit-command-line w/Functions/Zle/edit-command-line
> index 353f2609a..5c82e823a 100644
> --- i/Functions/Zle/edit-command-line
> +++ w/Functions/Zle/edit-command-line
> @@ -22 +22 @@
> -      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
> +      eval "${editor[@]} -c 'normal! ${byteoffset}go' -- $1";;
> @@ -25,2 +25,2 @@
> -      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
> -    (*) ${=editor} $1;;
> +      eval "${editor[@]} +${#lines}:$((${#lines[-1]} + 1)) $1";;
> +    (*) eval "${editor[@]} $1";;

LGTM, except that $1 should be changed into ${(q)1} due to the eval.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  0:07 ` Daniel Shahaf
@ 2017-10-14  0:45   ` Marco Hinz
  2017-10-14  0:55   ` Bart Schaefer
  1 sibling, 0 replies; 11+ messages in thread
From: Marco Hinz @ 2017-10-14  0:45 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

Ah, right. Thanks!

> On 14. Oct 2017, at 02:07, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> 
> Marco Hinz wrote on Sat, 14 Oct 2017 01:05 +0200:
>> diff --git i/Functions/Zle/edit-command-line w/Functions/Zle/edit-command-line
>> index 353f2609a..5c82e823a 100644
>> --- i/Functions/Zle/edit-command-line
>> +++ w/Functions/Zle/edit-command-line
>> @@ -22 +22 @@
>> -      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
>> +      eval "${editor[@]} -c 'normal! ${byteoffset}go' -- $1";;
>> @@ -25,2 +25,2 @@
>> -      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
>> -    (*) ${=editor} $1;;
>> +      eval "${editor[@]} +${#lines}:$((${#lines[-1]} + 1)) $1";;
>> +    (*) eval "${editor[@]} $1";;
> 
> LGTM, except that $1 should be changed into ${(q)1} due to the eval.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  0:07 ` Daniel Shahaf
  2017-10-14  0:45   ` Marco Hinz
@ 2017-10-14  0:55   ` Bart Schaefer
  2017-10-14  1:52     ` Daniel Shahaf
                       ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Bart Schaefer @ 2017-10-14  0:55 UTC (permalink / raw)
  To: zsh-workers

On Oct 14, 12:07am, Daniel Shahaf wrote:
} Subject: Re: [PATCH] edit-command-line breaks arguments with spaces
}
} Marco Hinz wrote on Sat, 14 Oct 2017 01:05 +0200:
} > +      eval "${editor[@]} +${#lines}:$((${#lines[-1]} + 1)) $1";;
} > +    (*) eval "${editor[@]} $1";;
} 
} LGTM, except that $1 should be changed into ${(q)1} due to the eval.

Wouldn't this work and be preferable to eval?


diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 353f260..f77eb35 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -15,15 +15,15 @@
   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[2]
 
   # Open the editor, placing the cursor at the right place if we know how.
-  local editor=${${VISUAL:-${EDITOR:-vi}}}
+  local editor=(${(Q)${(z)${VISUAL:-${EDITOR:-vi}}}})
   case $editor in 
     (*vim*)
       integer byteoffset=$(( $#PREBUFFER + $#LBUFFER + 1 ))
-      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
+      ${editor} -c "normal! ${byteoffset}go" -- $1;;
     (*emacs*)
       local lines=( ${(f):-"$PREBUFFER$LBUFFER"} )
-      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
-    (*) ${=editor} $1;;
+      ${editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
+    (*) ${editor} $1;;
   esac
 
   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[1]

-- 
Barton E. Schaefer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  0:55   ` Bart Schaefer
@ 2017-10-14  1:52     ` Daniel Shahaf
  2017-10-14  4:21       ` Phil Pennock
  2017-10-14  1:56     ` Daniel Shahaf
  2017-10-14 18:40     ` Marco Hinz
  2 siblings, 1 reply; 11+ messages in thread
From: Daniel Shahaf @ 2017-10-14  1:52 UTC (permalink / raw)
  To: zsh-workers

Bart Schaefer wrote on Fri, 13 Oct 2017 17:55 -0700:
> On Oct 14, 12:07am, Daniel Shahaf wrote:
> } Subject: Re: [PATCH] edit-command-line breaks arguments with spaces
> }
> } Marco Hinz wrote on Sat, 14 Oct 2017 01:05 +0200:
> } > +      eval "${editor[@]} +${#lines}:$((${#lines[-1]} + 1)) $1";;
> } > +    (*) eval "${editor[@]} $1";;
> } 
> } LGTM, except that $1 should be changed into ${(q)1} due to the eval.
> 
> Wouldn't this work and be preferable to eval?

If $EDITOR is set to "vim -c ''", then:

- current master passes «''» (two characters) for vim's argv[2]

- using eval passes the empty string for vim's argv[2]

- using ${(Q)} elides the "" entirely

So this edge case seems to favour eval.

Come to think of it, using eval will make $EDITOR subject to alias expansions.

> diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
> index 353f260..f77eb35 100644
> --- a/Functions/Zle/edit-command-line
> +++ b/Functions/Zle/edit-command-line
> @@ -15,15 +15,15 @@
>    (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[2]
>  
>    # Open the editor, placing the cursor at the right place if we know how.
> -  local editor=${${VISUAL:-${EDITOR:-vi}}}
> +  local editor=(${(Q)${(z)${VISUAL:-${EDITOR:-vi}}}})
>    case $editor in 
>      (*vim*)
>        integer byteoffset=$(( $#PREBUFFER + $#LBUFFER + 1 ))
> -      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
> +      ${editor} -c "normal! ${byteoffset}go" -- $1;;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  0:55   ` Bart Schaefer
  2017-10-14  1:52     ` Daniel Shahaf
@ 2017-10-14  1:56     ` Daniel Shahaf
  2017-10-14 18:40     ` Marco Hinz
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2017-10-14  1:56 UTC (permalink / raw)
  To: zsh-workers

Bart Schaefer wrote on Fri, 13 Oct 2017 17:55 -0700:
>      (*emacs*)
>        local lines=( ${(f):-"$PREBUFFER$LBUFFER"} )

Separate question: shouldn't this be changed from «${(f):-"…"}» to
«"${(@f):-"…"}"» in order to DTRT when one of the lines in $PREBUFFER is empty?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  1:52     ` Daniel Shahaf
@ 2017-10-14  4:21       ` Phil Pennock
  2017-10-14 13:57         ` Daniel Shahaf
  0 siblings, 1 reply; 11+ messages in thread
From: Phil Pennock @ 2017-10-14  4:21 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On 2017-10-14 at 01:52 +0000, Daniel Shahaf wrote:
> If $EDITOR is set to "vim -c ''", then:
> 
> - current master passes «''» (two characters) for vim's argv[2]
> 
> - using eval passes the empty string for vim's argv[2]
> 
> - using ${(Q)} elides the "" entirely

No, the (Q) has nothing to do with it.  The problem is that in zsh,
expanding an unquoted array variable omits empty elements.

So instead of «$editor» it needs to be «"${editor[@]}"» or
«"${(@)editor}"» to keep the empty elements present at _expansion_ time.

This is the same reason that even in zsh, in normal zsh mode of not
splitting on whitespace, when passing through remaining elements of argv
unmolested you still have to use «"$@"» instead of just «$@».

  % FOO="vim -C ''"
  % foo=("${(Q@)${(z)FOO}}")
  % print $#foo
  3
  % print -l $foo
  vim
  -C
  % print -l .$foo
  .vim
  .-C
  .
  % print -l "${(@)foo}"
  vim
  -C

  %

-Phil


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  4:21       ` Phil Pennock
@ 2017-10-14 13:57         ` Daniel Shahaf
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Shahaf @ 2017-10-14 13:57 UTC (permalink / raw)
  To: zsh-workers

Phil Pennock wrote on Sat, 14 Oct 2017 00:21 -0400:
> On 2017-10-14 at 01:52 +0000, Daniel Shahaf wrote:
> > If $EDITOR is set to "vim -c ''", then:
> > 
> > - current master passes «''» (two characters) for vim's argv[2]
> > 
> > - using eval passes the empty string for vim's argv[2]
> > 
> > - using ${(Q)} elides the "" entirely
> 
> No, the (Q) has nothing to do with it.  The problem is that in zsh,
> expanding an unquoted array variable omits empty elements.
> 
> So instead of «$editor» it needs to be «"${editor[@]}"» or
> «"${(@)editor}"» to keep the empty elements present at _expansion_ time.

Good point; changing «${(Q)foo}» to «"${(@Q)foo}"» and «$editor» to «"${editor[@]}"»
would also work: the problem wasn't (Q) per se, but the lack of (@).

Between eval and (@Q) I think I'm leaning towards the latter.

Thanks Phil.

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14  0:55   ` Bart Schaefer
  2017-10-14  1:52     ` Daniel Shahaf
  2017-10-14  1:56     ` Daniel Shahaf
@ 2017-10-14 18:40     ` Marco Hinz
  2017-10-15  1:35       ` Bart Schaefer
  2 siblings, 1 reply; 11+ messages in thread
From: Marco Hinz @ 2017-10-14 18:40 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers


> Wouldn't this work and be preferable to eval?
> 
> diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
> index 353f260..f77eb35 100644
> --- a/Functions/Zle/edit-command-line
> +++ b/Functions/Zle/edit-command-line
> @@ -15,15 +15,15 @@
>   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[2]
> 
>   # Open the editor, placing the cursor at the right place if we know how.
> -  local editor=${${VISUAL:-${EDITOR:-vi}}}
> +  local editor=(${(Q)${(z)${VISUAL:-${EDITOR:-vi}}}})
>   case $editor in 
>     (*vim*)
>       integer byteoffset=$(( $#PREBUFFER + $#LBUFFER + 1 ))
> -      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
> +      ${editor} -c "normal! ${byteoffset}go" -- $1;;
>     (*emacs*)
>       local lines=( ${(f):-"$PREBUFFER$LBUFFER"} )
> -      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
> -    (*) ${=editor} $1;;
> +      ${editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
> +    (*) ${editor} $1;;
>   esac
> 
>   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[1]

Oh, wow. Splitting words but taking quoted arguments into account.. I was looking for something like (z).

The patch works well and I'd prefer that approach.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-14 18:40     ` Marco Hinz
@ 2017-10-15  1:35       ` Bart Schaefer
  2017-10-15 14:15         ` Marco Hinz
  0 siblings, 1 reply; 11+ messages in thread
From: Bart Schaefer @ 2017-10-15  1:35 UTC (permalink / raw)
  To: zsh-workers

On Oct 14,  8:40pm, Marco Hinz wrote:
}
} Oh, wow. Splitting words but taking quoted arguments into account.. I
} was looking for something like (z).
}
} The patch works well and I'd prefer that approach.

The following seems to be the full set of agreed-upon changes.


diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 353f260..e17893e 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -15,15 +15,15 @@
   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[2]
 
   # Open the editor, placing the cursor at the right place if we know how.
-  local editor=${${VISUAL:-${EDITOR:-vi}}}
+  local editor=( "${(@Q)${(z)${VISUAL:-${EDITOR:-vi}}}}" )
   case $editor in 
     (*vim*)
       integer byteoffset=$(( $#PREBUFFER + $#LBUFFER + 1 ))
-      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
+      "${(@)editor}" -c "normal! ${byteoffset}go" -- $1;;
     (*emacs*)
-      local lines=( ${(f):-"$PREBUFFER$LBUFFER"} )
-      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
-    (*) ${=editor} $1;;
+      local lines=( "${(@f):-"$PREBUFFER$LBUFFER"}" )
+      "${(@)editor}" +${#lines}:$((${#lines[-1]} + 1)) $1;;
+    (*) "${(@)editor}" $1;;
   esac
 
   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[1]


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] edit-command-line breaks arguments with spaces
  2017-10-15  1:35       ` Bart Schaefer
@ 2017-10-15 14:15         ` Marco Hinz
  0 siblings, 0 replies; 11+ messages in thread
From: Marco Hinz @ 2017-10-15 14:15 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

Perfect!

> The following seems to be the full set of agreed-upon changes.
> 
> 
> diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
> index 353f260..e17893e 100644
> --- a/Functions/Zle/edit-command-line
> +++ b/Functions/Zle/edit-command-line
> @@ -15,15 +15,15 @@
>   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[2]
> 
>   # Open the editor, placing the cursor at the right place if we know how.
> -  local editor=${${VISUAL:-${EDITOR:-vi}}}
> +  local editor=( "${(@Q)${(z)${VISUAL:-${EDITOR:-vi}}}}" )
>   case $editor in 
>     (*vim*)
>       integer byteoffset=$(( $#PREBUFFER + $#LBUFFER + 1 ))
> -      ${=editor} -c "normal! ${byteoffset}go" -- $1;;
> +      "${(@)editor}" -c "normal! ${byteoffset}go" -- $1;;
>     (*emacs*)
> -      local lines=( ${(f):-"$PREBUFFER$LBUFFER"} )
> -      ${=editor} +${#lines}:$((${#lines[-1]} + 1)) $1;;
> -    (*) ${=editor} $1;;
> +      local lines=( "${(@f):-"$PREBUFFER$LBUFFER"}" )
> +      "${(@)editor}" +${#lines}:$((${#lines[-1]} + 1)) $1;;
> +    (*) "${(@)editor}" $1;;
>   esac
> 
>   (( $+zle_bracketed_paste )) && print -r -n - $zle_bracketed_paste[1]


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-10-15 14:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-13 23:05 [PATCH] edit-command-line breaks arguments with spaces Marco Hinz
2017-10-14  0:07 ` Daniel Shahaf
2017-10-14  0:45   ` Marco Hinz
2017-10-14  0:55   ` Bart Schaefer
2017-10-14  1:52     ` Daniel Shahaf
2017-10-14  4:21       ` Phil Pennock
2017-10-14 13:57         ` Daniel Shahaf
2017-10-14  1:56     ` Daniel Shahaf
2017-10-14 18:40     ` Marco Hinz
2017-10-15  1:35       ` Bart Schaefer
2017-10-15 14:15         ` Marco Hinz

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).