Re: [PATCH?] Nofork and removing newlines

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Tue, Mar 12, 2024 at 4:19 PM Oliver Kiddle <opk@xxxxxxx> wrote:
>
> > Bart Schaefer wrote:
> > > See above about the requirement for it to look like ${|ident|...}.
> > > Since = * and ? are not identifiers, this is like writing { =|: } etc.
>
> Considering this explanation, it is apparent that allowing |ident| is
> not fully compatible with mksh where ${|ls| cat -} runs ls.

Hm, yes.  Although I wasn't really aiming for compatibility, rather
for borrowing the idea (via Sebastian's original attempt at it).  I
was also I confess a bit stuck on the idea that every case would look
like ${|REPLY=...} when of course piping to "read" etc. are also valid
ways to assign to REPLY.  How often would there be a command name with
no arguments in that position?

> And this leads on to the later question as we probably don't want to
> expand considerably on what is valid between the vertical bars.

I hesitate in suggesting this, but ... is there any existing case in
which "${{" is valid?  If not, I think I can change ${|var|...} to be
${{var}...} without too much violence (except to the doc, bleah).

> Yes that works. Is nice to see namerefs coming up in nifty solutions. I
> hadn't checked the code for what supporting ? / ! would involve.

Mostly it involves rejiggering valid_refname() to behave more like
itype_end(), if you mean supporting e.g. ${|?|...}.

> If trivial why not, but I well understand not wanting to do anything
> that involves the lexer.

That (and using {var} instead of |var|) would except for a single
conditional test all happen in subst.c, the lexer already skips ahead.

> Bart Schaefer wrote:
> > [...] I could investigate
> > whether we could do things like ${|=|...} is the same as ${=${ ... }},
> > ${|~|...} is ${~${ ... }}, etc.  That only saves 1 character, though,
> > and I'm not sure it's clearer.
>
> Would that potentially also extend to something like ${|=var| ... }

It could, yes.

> That might look like a default value assignment to someone

Would ${{=var}...} look better?  The doubled braces do give me pause.

> Given that the ${|var| ... } form appears to create a function-like
> scope, should var perhaps be auto-declared local for that scope and the
> local value be substituted?

I considered that but
(a) the implementation is messy, as the state of the parameter scope
has to be carried around subst.c a lot longer than with the single
known scalar "REPLY"
(b) it diverges even farther from the idea that REPLY is a
semi-special thing -- note that REPLY is automatically saved and
restored around ${|... REPLY=...}
(c) creating it local doesn't really add much that you can't do with
${ local value; ... } and
(d) part of the point was to be able to push the variable up to the
caller as a side effect, so you don't have to write
  value=${|value| ... value=...}
although I guess you do have to declare it somewhere so that's not
entirely helpful.

> The quoting approach is clean and logical and is probably my preferred
> option.  [...]  consistency with ${var} is perhaps more
> important - it does word splitting based on the shwordsplit option.

Thanks for the vote.

> > Separately, it's definitely possible to make zsh-mode ${ ... } trim
> > only one newline instead of all of them.
>
> Only one is probably the most useful. I would mostly associate the fact
> that $(...) strips multiple with the fact that it does word splitting

This is the code diff to make emulation trim all, ${ ... } trim one,
"${ ... }" trim none ... not re-doing the doc diff yet.

diff --git a/Src/subst.c b/Src/subst.c
index 49f7336bb..9d20a2d0e 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1900,6 +1900,7 @@ paramsubst(LinkList l, LinkNode n, char **str,
int qt, int pf_flags,
        /* The command string to be run by ${|...;} */
        char *cmdarg = NULL;
        size_t slen = 0;
+       int trim = (!EMULATION(EMULATE_ZSH)) ? 2 : !qt;
        inbrace = 1;
        s++;

@@ -2005,10 +2006,13 @@ paramsubst(LinkList l, LinkNode n, char **str,
int qt, int pf_flags,
                int onoerrs = noerrs, rplylen;
                noerrs = 2;
                rplylen = zstuff(&cmdarg, rplytmp);
-               if (! EMULATION(EMULATE_ZSH)) {
+               if (trim) {
                    /* bash and ksh strip trailing newlines here */
-                   while (rplylen > 0 && cmdarg[rplylen-1] == '\n')
+                   while (rplylen > 0 && cmdarg[rplylen-1] == '\n') {
                        rplylen--;
+                       if (trim == 1)
+                           break;
+                   }
                    cmdarg[rplylen] = 0;
                }
                noerrs = onoerrs;