zsh-workers
 help / color / mirror / code / Atom feed
* [patch] Fix VCS_INFO_reposub's whitespace handling
@ 2014-09-28 17:10 Marco Hinz
  2014-09-28 17:39 ` Peter Stephenson
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Marco Hinz @ 2014-09-28 17:10 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

Hi,

VCS_INFO_reposub is the function called to process VCS_INFO's %S flag
in 'formats' and 'actionformats'.

Problem:

$ mkdir -p '/tmp/foo   bar'
$ cd !$
$ VCS_INFO_reposub

Output:

"tmp/foobar" instead of "tmp/foo   bar".

I'm new to zsh internals, but apparently something causes the string
'foo   bar' to be read in as 2 arguments. printf only takes 1
argument, and so 'bar' just gets appended.

Could someone point me to the appropriate parameter expansion rule?

Solution:

Double-quote ${$(pwd -P)#$base/}.

Regards,

Marco

[-- Attachment #2: reposub.patch --]
[-- Type: text/x-diff, Size: 349 bytes --]

diff --git a/Functions/VCS_Info/VCS_INFO_reposub b/Functions/VCS_Info/VCS_INFO_reposub
index 1c16f0e..0e6a8b0 100644
--- a/Functions/VCS_Info/VCS_INFO_reposub
+++ b/Functions/VCS_Info/VCS_INFO_reposub
@@ -9,5 +9,5 @@ local base=${1%%/##}
     printf '.'
     return 1
 }
-printf '%s' ${$(pwd -P)#$base/}
+printf '%s' "${$(pwd -P)#$base/}"
 return 0

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

* Re: [patch] Fix VCS_INFO_reposub's whitespace handling
  2014-09-28 17:10 [patch] Fix VCS_INFO_reposub's whitespace handling Marco Hinz
@ 2014-09-28 17:39 ` Peter Stephenson
  2014-09-28 18:52 ` Bart Schaefer
  2014-09-29 12:23 ` [PATCH] Fix VCS_INFO_reposub's command expansion Frank Terbeck
  2 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2014-09-28 17:39 UTC (permalink / raw)
  To: zsh-workers

On Sun, 28 Sep 2014 19:10:33 +0200
Marco Hinz <mh.codebro@gmail.com> wrote:
> Could someone point me to the appropriate parameter expansion rule?

In

${$(pwd -P)#$base/}

you're hitting the special rule applied to the command substitution
rather than the parameter substitution.

Zsh is actually less likely (by default) to split substitutions than
other shells, but for $(...) it works the same way other shells do.  You
wouldn't hit this just with parameter substitution unless you had the
option SH_WORD_SPLIT set.  See the section COMMAND SUBSTITUTION in the
zshexpn manual page.  The second sentence is the exact rule.

The only effect parameter substitution has on the result is the explicit
one, i.e. removing $base/ from the head of the nested result.  If you
want the full hairy list of parameter rules (hint: you don't) search for
"brain damage" in zshexpn.

For nested operations involving command substitions, the description is
a couple of paragraphs above "Parameter Expansion Flags".  The following
paragraph, indicating you can double-quote just the nested command
substitution, is also relevant (your patch looks fine as it is since
we're dealing throughout with one word).

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/


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

* Re: [patch] Fix VCS_INFO_reposub's whitespace handling
  2014-09-28 17:10 [patch] Fix VCS_INFO_reposub's whitespace handling Marco Hinz
  2014-09-28 17:39 ` Peter Stephenson
@ 2014-09-28 18:52 ` Bart Schaefer
  2014-09-29 12:23 ` [PATCH] Fix VCS_INFO_reposub's command expansion Frank Terbeck
  2 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2014-09-28 18:52 UTC (permalink / raw)
  To: zsh-workers; +Cc: Marco Hinz

On Sep 28,  7:10pm, Marco Hinz wrote:
}
} I'm new to zsh internals, but apparently something causes the string
} 'foo   bar' to be read in as 2 arguments. printf only takes 1
} argument, and so 'bar' just gets appended.
} 
} Could someone point me to the appropriate parameter expansion rule?
} 
} Solution:
} 
} Double-quote ${$(pwd -P)#$base/}.

It's the $(pwd -P) that's the problem; $(...) is subject to $IFS split
unless quoted.  Section 14.4 "Command Substitution" in the docs.


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

* [PATCH] Fix VCS_INFO_reposub's command expansion
  2014-09-28 17:10 [patch] Fix VCS_INFO_reposub's whitespace handling Marco Hinz
  2014-09-28 17:39 ` Peter Stephenson
  2014-09-28 18:52 ` Bart Schaefer
@ 2014-09-29 12:23 ` Frank Terbeck
  2014-09-29 13:26   ` Peter Stephenson
  2 siblings, 1 reply; 5+ messages in thread
From: Frank Terbeck @ 2014-09-29 12:23 UTC (permalink / raw)
  To: zsh-workers; +Cc: Marco Hinz

Reported-by: Marco Hinz <mh.codebro@gmail.com>
---

The problem, as others mentioned, is that $(...) is subject
to word-splitting. I think we should fix it like is done
below. Even though in a variable assignment:

  foo=$(...)

There is *no* word-splitting, I think the use of the double quotes
is warranted, since it underlines the intend. Also, using a
temporary variable like this should save a stat() call.

 Functions/VCS_Info/VCS_INFO_reposub | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Functions/VCS_Info/VCS_INFO_reposub b/Functions/VCS_Info/VCS_INFO_reposub
index 1c16f0e..8ebc90b 100644
--- a/Functions/VCS_Info/VCS_INFO_reposub
+++ b/Functions/VCS_Info/VCS_INFO_reposub
@@ -3,11 +3,12 @@
 ## Distributed under the same BSD-ish license as zsh itself.
 
 setopt localoptions extendedglob NO_shwordsplit
-local base=${1%%/##}
+local base=${1%%/##} tmp
 
-[[ $(pwd -P) == ${base}/* ]] || {
+tmp="$(pwd -P)"
+[[ $tmp == ${base}/* ]] || {
     printf '.'
     return 1
 }
-printf '%s' ${$(pwd -P)#$base/}
+printf '%s' ${tmp#$base/}
 return 0
-- 
2.1.0.60.g85f0837


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

* Re: [PATCH] Fix VCS_INFO_reposub's command expansion
  2014-09-29 12:23 ` [PATCH] Fix VCS_INFO_reposub's command expansion Frank Terbeck
@ 2014-09-29 13:26   ` Peter Stephenson
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2014-09-29 13:26 UTC (permalink / raw)
  To: zsh-workers

On Mon, 29 Sep 2014 14:23:28 +0200
Frank Terbeck <ft@bewatermyfriend.org> wrote:
> -[[ $(pwd -P) == ${base}/* ]] || {
> +tmp="$(pwd -P)"
> +[[ $tmp == ${base}/* ]] || {

There's no problem with this change, but actually you get away with it
anyway here since the [[ ... ]] context knows it should expand as a
single word.

[[ $(print eurgh yuk) == "eurgh yuk" ]] && print OK

pws


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

end of thread, other threads:[~2014-09-29 13:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-28 17:10 [patch] Fix VCS_INFO_reposub's whitespace handling Marco Hinz
2014-09-28 17:39 ` Peter Stephenson
2014-09-28 18:52 ` Bart Schaefer
2014-09-29 12:23 ` [PATCH] Fix VCS_INFO_reposub's command expansion Frank Terbeck
2014-09-29 13:26   ` Peter Stephenson

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