GNU bug report logs - #59622
29.0.50; [PATCH] Regression in Eshell's handling of escaped newlines

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59622 in the body.
You can then email your comments to 59622 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; [PATCH] Regression in Eshell's handling of escaped newlines
Date: Sat, 26 Nov 2022 16:36:59 -0800

[Message part 1 (text/plain, inline)]

Starting from "emacs -Q -f eshell":

  # Emacs 28
  ~ $ echo foo\
  bar

  foobar

  # Emacs 29
  ~ $ echo foo\
  bar

  foo
  bar

That is, Emacs 28 used to treat escaped newlines in the way you'd expect 
from other shells: it expands to the empty string. Now in Emacs 29, it 
inserts a literal newline.

There's also a similar bug when doing this inside double-quotes:

  # Emacs 28
  ~ $ echo "foo\
  bar"

  ("foo\\" "bar")

  # Emacs 29
  ~  $ echo "foo\
  bar"

  foo\
  bar

Here, both cases are wrong. In Emacs 29, it inserts the literal 
backslash+newline combo, but Emacs 28 is even worse: it returns a list 
of two elements! Wrapping arguments in quotes should always produce a 
string in Eshell.

[0001-Treat-escaped-newlines-in-Eshell-as-the-empty-string.patch (text/plain, attachment)]

Message #8 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Sat, 26 Nov 2022 16:42:40 -0800

On 11/26/2022 4:36 PM, Jim Porter wrote:
>    ~ $ echo foo\
>    bar
[snip]
>    ~ $ echo "foo\
>    bar"

Sorry, I forgot to mention: the new behavior with my patch in both cases 
above is to print "foobar" as a single word.

Message #11 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: 59622 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Sat, 3 Dec 2022 17:41:50 -0800

On 11/26/2022 4:36 PM, Jim Porter wrote:
> Starting from "emacs -Q -f eshell":
> 
>    # Emacs 28
>    ~ $ echo foo\
>    bar
> 
>    foobar
> 
>    # Emacs 29
>    ~ $ echo foo\
>    bar
> 
>    foo
>    bar
> 
> That is, Emacs 28 used to treat escaped newlines in the way you'd expect 
> from other shells: it expands to the empty string. Now in Emacs 29, it 
> inserts a literal newline.

Eli, since this is a regression from Emacs 28 (likely fallout from one 
of my changes to fix some longstanding bugs with quotes in Eshell), 
would my current patch be ok on the release branch? I can try to 
minimize the changes a bit further (I slightly refactored 
'eshell-parse-backslash' to reduce repetition), but since it has unit 
tests, I think it should be pretty safe either way.

Message #14 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Sun, 04 Dec 2022 09:26:04 +0200

> Date: Sat, 3 Dec 2022 17:41:50 -0800
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 11/26/2022 4:36 PM, Jim Porter wrote:
> > Starting from "emacs -Q -f eshell":
> > 
> >    # Emacs 28
> >    ~ $ echo foo\
> >    bar
> > 
> >    foobar
> > 
> >    # Emacs 29
> >    ~ $ echo foo\
> >    bar
> > 
> >    foo
> >    bar
> > 
> > That is, Emacs 28 used to treat escaped newlines in the way you'd expect 
> > from other shells: it expands to the empty string. Now in Emacs 29, it 
> > inserts a literal newline.
> 
> Eli, since this is a regression from Emacs 28 (likely fallout from one 
> of my changes to fix some longstanding bugs with quotes in Eshell), 
> would my current patch be ok on the release branch?

Yes, but please do try to make it as safe as is feasible.

> I can try to minimize the changes a bit further (I slightly refactored
> 'eshell-parse-backslash' to reduce repetition), but since it has unit
> tests, I think it should be pretty safe either way.

OK.

Message #17 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Sun, 4 Dec 2022 17:35:30 -0800

[Message part 1 (text/plain, inline)]

On 12/3/2022 11:26 PM, Eli Zaretskii wrote:
>> Date: Sat, 3 Dec 2022 17:41:50 -0800
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>> Eli, since this is a regression from Emacs 28 (likely fallout from one
>> of my changes to fix some longstanding bugs with quotes in Eshell),
>> would my current patch be ok on the release branch?
> 
> Yes, but please do try to make it as safe as is feasible.

Thanks. How does this look? I just simplified the change in 
'eshell-parse-backslash' so that the only difference is an extra 
conditional (plus whitespace changes).

[0001-Treat-escaped-newlines-in-Eshell-as-the-empty-string.patch (text/plain, attachment)]

Message #20 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Mon, 05 Dec 2022 14:39:06 +0200

> Date: Sun, 4 Dec 2022 17:35:30 -0800
> Cc: 59622 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 12/3/2022 11:26 PM, Eli Zaretskii wrote:
> >> Date: Sat, 3 Dec 2022 17:41:50 -0800
> >> From: Jim Porter <jporterbugs <at> gmail.com>
> >>
> >> Eli, since this is a regression from Emacs 28 (likely fallout from one
> >> of my changes to fix some longstanding bugs with quotes in Eshell),
> >> would my current patch be ok on the release branch?
> > 
> > Yes, but please do try to make it as safe as is feasible.
> 
> Thanks. How does this look? I just simplified the change in 
> 'eshell-parse-backslash' so that the only difference is an extra 
> conditional (plus whitespace changes).

Looks good, thanks.  One comment:

> +When you escape a character with @code{\} outside of quotes, the
> +result is the literal character immediately following it, so
> +@samp{\$10} means the literal string @code{$10}.  Inside of
> +double quotes, the result is the literal character following it if
> +that character is special, or the full @code{\@var{c}} sequence
> +otherwise; inside double-quotes, @code{\}, @code{"}, and @code{$} are
> +considered special.

The last sentence is very unclear, please try saying what you need in a
clearer way, and/or maybe add a couple of examples.

Message #23 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Tue, 6 Dec 2022 20:18:30 -0800

[Message part 1 (text/plain, inline)]

On 12/5/2022 4:39 AM, Eli Zaretskii wrote:
>> +When you escape a character with @code{\} outside of quotes, the
>> +result is the literal character immediately following it, so
>> +@samp{\$10} means the literal string @code{$10}.  Inside of
>> +double quotes, the result is the literal character following it if
>> +that character is special, or the full @code{\@var{c}} sequence
>> +otherwise; inside double-quotes, @code{\}, @code{"}, and @code{$} are
>> +considered special.
> 
> The last sentence is very unclear, please try saying what you need in a
> clearer way, and/or maybe add a couple of examples.

Ok, I expanded that part of the manual and added a few more examples. 
How does this look?

I also split the patch in two: the first patch for the 29 branch (which 
makes the least code changes I could manage), and the second as an 
additional small cleanup patch for master only.

[29.1--0001-Treat-escaped-newlines-in-Eshell-as-the-empty-string.patch (text/plain, attachment)]

[master--0002-Reorganize-the-conditionals-in-eshell-parse-backslas.patch (text/plain, attachment)]

Message #26 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Wed, 07 Dec 2022 15:34:36 +0200

> Date: Tue, 6 Dec 2022 20:18:30 -0800
> Cc: 59622 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> > The last sentence is very unclear, please try saying what you need in a
> > clearer way, and/or maybe add a couple of examples.
> 
> Ok, I expanded that part of the manual and added a few more examples. 
> How does this look?

Much better, thanks.

> I also split the patch in two: the first patch for the 29 branch (which 
> makes the least code changes I could manage), and the second as an 
> additional small cleanup patch for master only.

Since which Emacs version do we have this regression?  If it's a very
old regression, I'd prefer not to fix it on the emacs-29 branch.

>  @subsection Quoting and escaping
>  As with other shells, you can escape special characters and spaces
> -with by prefixing the character with a backslash (@code{\}), or by
> -surrounding the string with apostrophes (@code{''}) or double quotes
> -(@code{""}).  This is needed especially for file names with special
> -characters like pipe (@code{|}), which could be part of remote file
> +with by prefixing the character with a backslash (@samp{\}), or by

"with by prefixing" is a typo; probably "with" should be dropped.

Message #29 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Wed, 7 Dec 2022 09:57:59 -0800

On 12/7/2022 5:34 AM, Eli Zaretskii wrote:
>> Date: Tue, 6 Dec 2022 20:18:30 -0800
>> Cc: 59622 <at> debbugs.gnu.org
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>> I also split the patch in two: the first patch for the 29 branch (which
>> makes the least code changes I could manage), and the second as an
>> additional small cleanup patch for master only.
> 
> Since which Emacs version do we have this regression?  If it's a very
> old regression, I'd prefer not to fix it on the emacs-29 branch.

This worked properly in Emacs 28 and is broken in 29. I'm not 100% sure 
which patch regressed it, but my guess is patch 0003 in bug#54227, which 
was merged in March.

>>   @subsection Quoting and escaping
>>   As with other shells, you can escape special characters and spaces
>> -with by prefixing the character with a backslash (@code{\}), or by
>> -surrounding the string with apostrophes (@code{''}) or double quotes
>> -(@code{""}).  This is needed especially for file names with special
>> -characters like pipe (@code{|}), which could be part of remote file
>> +with by prefixing the character with a backslash (@samp{\}), or by
> 
> "with by prefixing" is a typo; probably "with" should be dropped.

Good catch! I missed that despite reading it a few times while editing 
the manual.

Message #32 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Wed, 07 Dec 2022 20:10:05 +0200

> Date: Wed, 7 Dec 2022 09:57:59 -0800
> Cc: 59622 <at> debbugs.gnu.org
> From: Jim Porter <jporterbugs <at> gmail.com>
> 
> On 12/7/2022 5:34 AM, Eli Zaretskii wrote:
> >> Date: Tue, 6 Dec 2022 20:18:30 -0800
> >> Cc: 59622 <at> debbugs.gnu.org
> >> From: Jim Porter <jporterbugs <at> gmail.com>
> >>
> >> I also split the patch in two: the first patch for the 29 branch (which
> >> makes the least code changes I could manage), and the second as an
> >> additional small cleanup patch for master only.
> > 
> > Since which Emacs version do we have this regression?  If it's a very
> > old regression, I'd prefer not to fix it on the emacs-29 branch.
> 
> This worked properly in Emacs 28 and is broken in 29. I'm not 100% sure 
> which patch regressed it, but my guess is patch 0003 in bug#54227, which 
> was merged in March.

Then we must fix the regression on the emacs-29 branch.

Thanks.

Message #35 received at 59622 <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59622 <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Wed, 7 Dec 2022 21:47:23 -0800

On 12/7/2022 10:10 AM, Eli Zaretskii wrote:
>> Date: Wed, 7 Dec 2022 09:57:59 -0800
>> Cc: 59622 <at> debbugs.gnu.org
>> From: Jim Porter <jporterbugs <at> gmail.com>
>>
>> This worked properly in Emacs 28 and is broken in 29. I'm not 100% sure
>> which patch regressed it, but my guess is patch 0003 in bug#54227, which
>> was merged in March.
> 
> Then we must fix the regression on the emacs-29 branch.
> 
> Thanks.

Thanks, that sounds right to me too. I've merged this (just the first 
patch with the fix) to the emacs-29 branch as 
cf5ce14a6b442649426003fed8dc02ee7edca914.

I'll leave this open until I merge the small cleanup patch to master 
once my first patch has gotten there.

Message #40 received at 59622-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 59622-done <at> debbugs.gnu.org
Subject: Re: bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of
 escaped newlines
Date: Thu, 8 Dec 2022 16:59:53 -0800

On 12/7/2022 9:47 PM, Jim Porter wrote:
> I'll leave this open until I merge the small cleanup patch to master 
> once my first patch has gotten there.

Pushed my followup patch to master as 
8c01829c01ca81c990eadf34bc16794b65d62c70. Closing this now.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.