Comment (# char) behavior in the sub-shell

Comment (# char) behavior in the sub-shell

[Stanislav Seletskiy]

zsh version: 5.4.2

I'm heavy user of `#`-aliases, so I like to do stuff like `seq 1 10 #
9` to grep for `9` (`alias -g -- '#'='| grep'`).

It works as expected when I'm entering command in interactive mode
without sub-shell, like:

`$ seq 1 10 # 9` — works well, I see only `9` in the output.

But when I'm trying to use same alias in the sub-shell (e.g. inside
`$()`), it doesn't work anymore:

`$ echo $(seq 1 10 # 9)` — doesn't work, I see `1 2 3 ... 10` in the output.

It seems for whatever reason everything after `#` in the sub-shell is ignored.

I've tried to disable INTERACTIVE_COMMENTS option without any success.

`$ histchars='!^' ; echo $(seq 1 10 # 9)` produces expected output,
but it's obviously will break a lot of other things.

I think it's a bug, because user should see consistent behavior in the
same interactive session. It's easily reproducible with `zsh -df` and
alias definition as I wrote above.

Re: Comment (# char) behavior in the sub-shell

[Bart Schaefer]

On Sep 9,  7:33pm, Stanislav Seletskiy wrote:
}
} It seems for whatever reason everything after `#` in the sub-shell is ignored.
} 
} I've tried to disable INTERACTIVE_COMMENTS option without any success.

A subshell is not an interactive shell, so INTERACTIVE_COMMENTS does not
make any difference here, except that it delays treating "#" as a comment
until after the subshell has begun.  In bash for example the "#" would
comment out the closing paren and your expression wouldn't work at all.

Also, because NO_INTERACTIVE_COMMENTS is the default, propagating it from
the command line into subshells would break many of the same things that
you are concerned about being broken by histchars='!^'.

-- 
Barton E. Schaefer

Re: Comment (# char) behavior in the sub-shell

[Peter Stephenson]

On Sat, 9 Sep 2017 19:33:59 +0300
Stanislav Seletskiy <s.seletskiy@gmail.com> wrote:
> zsh version: 5.4.2
> 
> I'm heavy user of `#`-aliases, so I like to do stuff like `seq 1 10 #
> 9` to grep for `9` (`alias -g -- '#'='| grep'`).
> 
> It works as expected when I'm entering command in interactive mode
> without sub-shell, like:
> 
> `$ seq 1 10 # 9` — works well, I see only `9` in the output.
> 
> But when I'm trying to use same alias in the sub-shell (e.g. inside
> `$()`), it doesn't work anymore:
> 
> `$ echo $(seq 1 10 # 9)` — doesn't work, I see `1 2 3 ... 10` in the output.

I suspect this changed some versions ago now.  At some point we changed
the way we handled $(...) to parse it better.  Previously, we just
blindly treated it as a string when we first enountered it, reading it
in interactively until we got to a closing parenthesis.  That wasn't
actually the right thing to do, and crucially it failed where there was
a case statement in the old syntax where a pattern only has a closing
parenthesis and not an opening parenthesis.  So what we do now is parse
properly until the (right) closing parenthesis, but still store what we
parsed as a string for reading again when we execute the $(...).

It so happens we don't keep the expanded aliases from the original parse
in the string we later replay.  There is a test for just this case;
here it is:

 alias fooalias=barexpansion
 funcwithalias() { echo $(fooalias); }
 functions funcwithalias
 barexpansion() { print This is the correct output.; }
 funcwithalias
0:Alias expanded in command substitution does not appear expanded in text
>funcwithalias () {
>	echo $(fooalias)
>}
>This is the correct output.

You'll see that the output from "functions" shown at the end contains
the unexpanded alias (that "0:..." in the middle simply gives the
expected exit status for the code above and a description).  In your
case, that would be the "#", and at the point where it finally does get
read in, in recent shell versions, it's treated like a string, not
interactive input.  If we kept expanded aliases the definition of
functionwithalias would change correspondingly.  Actually, there is an
argument for that since since that's what would happen for an alias in
the main body of the function, but I'm not sure how close the parallels
are

I'm not sure how much effort this is worth.

pws

Re: Comment (# char) behavior in the sub-shell

[Stanislav Seletskiy]

> I suspect this changed some versions ago now.  At some point we changed
> the way we handled $(...) to parse it better.  Previously, we just
> blindly treated it as a string when we first enountered it, reading it
> in interactively until we got to a closing parenthesis.  That wasn't
> actually the right thing to do, and crucially it failed where there was
> a case statement in the old syntax where a pattern only has a closing
> parenthesis and not an opening parenthesis.  So what we do now is parse
> properly until the (right) closing parenthesis, but still store what we
> parsed as a string for reading again when we execute the $(...).

So, shouldn't it be fixed? It's very misleading, since for me as for
user $(...) that I type in the shell is still interactive mode and
should be treated in the same way.

Re: Comment (# char) behavior in the sub-shell

[Bart Schaefer]

On Sep 10,  9:06pm, Peter Stephenson wrote:
}
} I suspect this changed some versions ago now.  At some point we changed
} the way we handled $(...) to parse it better.

I think this is much older than that, and has more to do with the fact
that comments are stripped out before aliases are expanded.  With no
aliasing at all:

torch% echo $ZSH_VERSION
4.2.0
torch% echo $(echo 1 10 # 9)
1 10

To demonstrate that it's not related to global aliasing:

torch% alias #='echo foo'
torch% #
foo
torch% echo X $(# 1 10) Y
X Y
torch% 

} It so happens we don't keep the expanded aliases from the original parse
} in the string we later replay.

If that were the only issue, then removing "#" from $histchars would not
"fix" the problem, I think?

} I'm not sure how much effort this is worth.

Using the comment delimiter as a global alias because interactive shells
don't recognize comments, certainly seems to me to be painting oneself
into a pretty tight corner case.

Still:

torch% echo $(echo $options[interactive])
on
torch% 

so one might ask why no_interactive_comments does not apply.

The following changes it, and all tests still pass, but I'm relatively
sure there is something or other that it breaks.

diff --git a/Src/exec.c b/Src/exec.c
index e2432fd..4230329 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4441,7 +4441,12 @@ getoutput(char *cmd, int qt)
     pid_t pid;
     char *s;
 
-    if (!(prog = parse_string(cmd, 0)))
+    int onc = nocomments;
+    nocomments = (interact && unset(INTERACTIVECOMMENTS));
+    prog = parse_string(cmd, 0);
+    nocomments = onc;
+
+    if (!prog)
 	return NULL;
 
     if ((s = simple_redir_name(prog, REDIR_READ))) {

Re: Comment (# char) behavior in the sub-shell

[Peter Stephenson]

On Sun, 10 Sep 2017 15:27:11 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Still:
> 
> torch% echo $(echo $options[interactive])
> on
> torch% 
> 
> so one might ask why no_interactive_comments does not apply.

For the record, it's this complicated test below.  If this evaluates to
true we strip comments.

    if (c == hashchar && !nocomments &&
	(isset(INTERACTIVECOMMENTS) ||
	 ((!lexflags || (lexflags & LEXFLAGS_COMMENTS)) && !expanding &&
	  (!interact || unset(SHINSTDIN) || strin)))) {

The difference between the first and second times through the $(...)
expansion is the second time "strin" is true.

Your patch could possibly do the right thing, as it hits the simpler
part of the test and shouldn't affect behaviour other than comments, so
it might be worth trying for now.

pws

Re: Comment (# char) behavior in the sub-shell

[Peter Stephenson]

On Mon, 11 Sep 2017 09:49:26 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Your patch could possibly do the right thing, as it hits the simpler
> part of the test and shouldn't affect behaviour other than comments, so
> it might be worth trying for now.

(Paranoia.)

I see, however, "nocomments" isn't saved and restored as we enter
different levels of reading history, which could be a problem.  But I
don't think so --- you're just modifying around *parsing* a string, not
execution of the result, so there's no possibility of the effect being
passed on where it shouldn't be.

Is there?

[The sombre background music to this post is available at all online
retailers.]

pws

Re: Comment (# char) behavior in the sub-shell

[Stanislav Seletskiy]

> (Paranoia.)
> 
> I see, however, "nocomments" isn't saved and restored as we enter
> different levels of reading history, which could be a problem.  But I
> don't think so --- you're just modifying around *parsing* a string, not
> execution of the result, so there's no possibility of the effect being
> passed on where it shouldn't be.
> 
> Is there?
> 
> [The sombre background music to this post is available at all online
> retailers.]

I'm using your patch and it works great.

Is there chance it will be merged into upstream? I really miss that
functionality.

Re: Comment (# char) behavior in the sub-shell

[Bart Schaefer]

On Sep 20,  6:54pm, Stanislav Seletskiy wrote:
}
} I'm using your patch and it works great.
} 
} Is there chance it will be merged into upstream? I really miss that
} functionality.

Here's an updated patch with a NEWS file entry.


diff --git a/NEWS b/NEWS
index 6847350..796a2c9 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,14 @@ CHANGES FROM PREVIOUS VERSIONS OF ZSH
 
 Note also the list of incompatibilities in the README file.
 
+Changes from 5.4 to 5.4.3
+-------------------------
+
+The effect of the NO_INTERACTIVE_COMMENTS option extends into $(...) and
+`...` command substitutions when used on the command line.  Previously,
+comments were always recognized within command substitutions unless the
+comment character "#" was disabled via reset of $histchars.
+
 Changes from 5.3.1 to 5.4
 -------------------------
 
diff --git a/Src/exec.c b/Src/exec.c
index d136766..31edfab 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4509,7 +4509,12 @@ getoutput(char *cmd, int qt)
     pid_t pid;
     char *s;
 
-    if (!(prog = parse_string(cmd, 0)))
+    int onc = nocomments;
+    nocomments = (interact && unset(INTERACTIVECOMMENTS));
+    prog = parse_string(cmd, 0);
+    nocomments = onc;
+
+    if (!prog)
 	return NULL;
 
     if ((s = simple_redir_name(prog, REDIR_READ))) {

Re: Comment (# char) behavior in the sub-shell

[Stanislav Seletskiy]

> On Sep 20,  6:54pm, Stanislav Seletskiy wrote:
> }
> } I'm using your patch and it works great.
> } 
> } Is there chance it will be merged into upstream? I really miss that
> } functionality.
> 
> Here's an updated patch with a NEWS file entry.

That's great news, thanks you!