PATCH: <<<: Document newline behavior and fix optimization

PATCH: <<<: Document newline behavior and fix optimization

[Mikael Magnusson]

The =(<<<foo) optimization forgot to add a newline, but =(cat<<<foo)
always did, make the behavior consistent, and document it.
---

This optimization was added in 2005 in workers/21758 (ad2bd42c858) and
has no motivation for excluding the newline, so I assume it's just a
minor mistake.
---
 Doc/Zsh/redirect.yo   | 3 ++-
 README                | 5 +++++
 Src/exec.c            | 4 +++-
 Test/A04redirect.ztst | 2 +-
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Doc/Zsh/redirect.yo b/Doc/Zsh/redirect.yo
index 2b48974b40..fd40ab5a4b 100644
--- a/Doc/Zsh/redirect.yo
+++ b/Doc/Zsh/redirect.yo
@@ -86,7 +86,8 @@ item(tt(<<<) var(word))(
 Perform shell expansion on var(word) and pass the result
 to standard input.  This is known as a em(here-string).
 Compare the use of var(word) in here-documents above, where var(word)
-does not undergo shell expansion.
+does not undergo shell expansion.  The result will have a trailing newline
+after it.
 )
 xitem(tt(<&) var(number))
 item(tt(>&) var(number))(
diff --git a/README b/README
index 353667289e..153f02b25b 100644
--- a/README
+++ b/README
@@ -33,6 +33,11 @@ details, see the documentation.
 Incompatibilities since 5.8
 ---------------------------
 
+The optimization for the =(<<<foo) construct with no command, which
+creates a temporary file with contents "foo", now adds a newline byte
+after "foo" for consistency with the behaviour of the <<< redirection
+everywhere else.
+
 PROMPT_SUBST expansion is no longer performed on arguments to prompt-
 expansion sequences such as %F.
 
diff --git a/Src/exec.c b/Src/exec.c
index f67074846f..70cbfc97fd 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4836,8 +4836,10 @@ getoutputfile(char *cmd, char **eptr)
 	singsub(&s);
 	if (errflag)
 	    s = NULL;
-	else
+	else {
 	    untokenize(s);
+	    s = dyncat(s, "\n");
+	}
     }
 
     if (!s)             /* Unclear why we need to do this before open() */
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index 993138e7dc..17f6dfa29a 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -440,7 +440,7 @@
 # This tests the here-string to filename optimisation; we can't
 # test that it's actually being optimised, but we can test that it
 # still works.
-  cat =(<<<$'This string has been replaced\nby a file containing it.\n')
+  cat =(<<<$'This string has been replaced\nby a file containing it.')
 0:Optimised here-string to filename
 >This string has been replaced
 >by a file containing it.
-- 
2.15.1

Re: PATCH: <<<: Document newline behavior and fix optimization

[Bart Schaefer]

On Tue, Mar 8, 2022 at 10:59 AM Mikael Magnusson <mikachu@gmail.com> wrote:
>
> The =(<<<foo) optimization forgot to add a newline, but =(cat<<<foo)
> always did, make the behavior consistent, and document it.

I regularly make use of the fact that <<<foo does not add a newline,
outside of substitution context.  I have no opinion on whether the
file created by =(<<<foo) should end in a newline, that's a different
matter.

Re: PATCH: <<<: Document newline behavior and fix optimization

[Bart Schaefer]

On Tue, Mar 8, 2022 at 11:38 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> I regularly make use of the fact that <<<foo does not add a newline

Or, hmm, I thought I did, but that can't be.  What am I thinking of?

Re: PATCH: <<<: Document newline behavior and fix optimization

[Stephane Chazelas]

2022-03-08 11:42:31 -0800, Bart Schaefer:
> On Tue, Mar 8, 2022 at 11:38 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > I regularly make use of the fact that <<<foo does not add a newline
> 
> Or, hmm, I thought I did, but that can't be.  What am I thinking of?

In Byron Rakitzis's clone of rc, <<< does not add a newline. I
heard here that that's the shell that introduced <<<, though
I've not found evidence of the fact that rc had it before zsh.

Maybe you were thinking of "$(<<<foo)" which strips trailing
newline*S* just like "$(echo foo)". The fact that it strips more
than one newline is a misfeature IMO though (shared by all
Bourne-like shells). That makes it that things like

dir=$(dirname -- "$file")

Are incorrect as they fail for files whose dirname ends in
newline characters (and why you want to use dir=$file:h instead
if you're lucky enough to use zsh).

I was thinking at some point maybe we could add a
$CMDSUB_RTRIM special variable, which one could set to $'\n' (or
$'(\r|)\n' for MSDOS users) instead of the default of $'\n#'
to allow users to fix that problem.

-- 
Stephane

Re: PATCH: <<<: Document newline behavior and fix optimization

[Stephane Chazelas]

2022-03-08 19:58:32 +0100, Mikael Magnusson:
> The =(<<<foo) optimization forgot to add a newline, but =(cat<<<foo)
> always did, make the behavior consistent, and document it.
[...]

The edit-command-line contrib widget may be relying on =(<<<xxx)
not adding a newline character.

-- 
Stephane

Re: PATCH: <<<: Document newline behavior and fix optimization

[Mikael Magnusson]

On 3/13/22, Stephane Chazelas <stephane@chazelas.org> wrote:
> 2022-03-08 19:58:32 +0100, Mikael Magnusson:
>> The =(<<<foo) optimization forgot to add a newline, but =(cat<<<foo)
>> always did, make the behavior consistent, and document it.
> [...]
>
> The edit-command-line contrib widget may be relying on =(<<<xxx)
> not adding a newline character.

The output from the editor is read with $() so I think it will cancel
out, either way. Just tested invoking it and pressing enter a bunch of
times, and was returned to an empty command line.

-- 
Mikael Magnusson

Re: PATCH: <<<: Document newline behavior and fix optimization

[Stephane Chazelas]

2022-03-13 20:28:35 +0100, Mikael Magnusson:
> On 3/13/22, Stephane Chazelas <stephane@chazelas.org> wrote:
> > 2022-03-08 19:58:32 +0100, Mikael Magnusson:
> >> The =(<<<foo) optimization forgot to add a newline, but =(cat<<<foo)
> >> always did, make the behavior consistent, and document it.
> > [...]
> >
> > The edit-command-line contrib widget may be relying on =(<<<xxx)
> > not adding a newline character.
> 
> The output from the editor is read with $() so I think it will cancel
> out, either way. Just tested invoking it and pressing enter a bunch of
> times, and was returned to an empty command line.
[...]

You're right sorry. I just came across the definition of that
widget and saw that =(<<<...) in it which reminded me of this
discussion. But I didn't look at it further.

If anything, your change would improved it, as at the moment, it
would likely launch a text editor on something that is not text
as it wouldn't end in a newline.

-- 
Stephane

Re: PATCH: <<<: Document newline behavior and fix optimization

[Jun T]

> 2022/03/09 3:58, Mikael Magnusson <mikachu@gmail.com> wrote:

> --- a/Test/A04redirect.ztst
> +++ b/Test/A04redirect.ztst
> @@ -440,7 +440,7 @@
> # This tests the here-string to filename optimisation; we can't
> # test that it's actually being optimised, but we can test that it
> # still works.
> -  cat =(<<<$'This string has been replaced\nby a file containing it.\n')
> +  cat =(<<<$'This string has been replaced\nby a file containing it.')

It seems this patch for A04redirect.ztst is not included in the commit.

Re: PATCH: <<<: Document newline behavior and fix optimization

[Mikael Magnusson]

On Wed, Mar 30, 2022 at 9:17 AM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>
>
> > 2022/03/09 3:58, Mikael Magnusson <mikachu@gmail.com> wrote:
>
> > --- a/Test/A04redirect.ztst
> > +++ b/Test/A04redirect.ztst
> > @@ -440,7 +440,7 @@
> > # This tests the here-string to filename optimisation; we can't
> > # test that it's actually being optimised, but we can test that it
> > # still works.
> > -  cat =(<<<$'This string has been replaced\nby a file containing it.\n')
> > +  cat =(<<<$'This string has been replaced\nby a file containing it.')
>
> It seems this patch for A04redirect.ztst is not included in the commit.

Ah, thanks. Somehow I don't have this hunk in my local repository at all,
even checking the reflog. Not sure how I managed that but I've committed
it now.

-- 
Mikael Magnusson