all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of escaped newlines
@ 2022-11-27  0:36 Jim Porter
  2022-11-27  0:42 ` Jim Porter
  2022-12-04  1:41 ` Jim Porter
  0 siblings, 2 replies; 12+ messages in thread
From: Jim Porter @ 2022-11-27  0:36 UTC (permalink / raw)
  To: 59622

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

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.

[-- Attachment #2: 0001-Treat-escaped-newlines-in-Eshell-as-the-empty-string.patch --]
[-- Type: text/plain, Size: 12014 bytes --]

From 3018c1c0c2d03c1754ec5cf12c836aa1c93ce468 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 26 Nov 2022 11:52:18 -0800
Subject: [PATCH] Treat escaped newlines in Eshell as the empty string

* lisp/eshell/esh-arg.el (eshell-parse-argument): Handle
'eshell-empty-token' as the result of an argument-parsing hook.
(eshell-parse-argument-hook): Document 'eshell-empty-token'.
(eshell-parse-backslash): Return 'eshell-empty-token' when
encountering an escaped newline.

* test/lisp/eshell/eshell-tests.el (eshell-test/escape-nonspecial)
(eshell-test/escape-nonspecial-unicode)
(eshell-test/escape-nonspecial-quoted)
(eshell-test/escape-special-quoted): Move from here...

* test/lisp/eshell/esh-arg-tests.el (esh-arg-test/escape/nonspecial)
(esh-arg-test/escape/nonspecial-unicode)
(esh-arg-test/escape-quoted/nonspecial)
(esh-arg-test/escape-quoted/special): ... to here.
(esh-arg-test/escape/special, esh-arg-test/escape/newline)
(esh-arg-test/escape-quoted/newline): New tests.

* doc/misc/eshell.texi (Arguments): Explain escaping logic in more
detail.
---
 doc/misc/eshell.texi              |  23 +++++++
 lisp/eshell/esh-arg.el            |  46 ++++++++-----
 test/lisp/eshell/esh-arg-tests.el | 105 ++++++++++++++++++++++++++++++
 test/lisp/eshell/eshell-tests.el  |  31 ---------
 4 files changed, 156 insertions(+), 49 deletions(-)
 create mode 100644 test/lisp/eshell/esh-arg-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index e6ddcf11df..67d8f8f81d 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -263,6 +263,29 @@ Arguments
 characters like pipe (@code{|}), which could be part of remote file
 names.
 
+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.
+
+Additionally, when escaping a newline, the whole escape sequence is
+removed by the parser.  This lets you continue commands across
+multiple lines:
+
+@example
+~ $ echo "foo\
+bar"
+foobar
+@end example
+
+Inside apostrophes, escaping works differently.  All characters
+between the apostrophes have their literal meaning except @code{'},
+which ends the quoted string.  To insert a literal apostrophe, you can
+use @code{''}.
+
 When using expansions (@pxref{Expansion}) in an Eshell command, the
 result may potentially be of any data type.  To ensure that the result
 is always a string, the expansion can be surrounded by double quotes.
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index f87cc2f20a..29c8b7ba0b 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -146,9 +146,10 @@ eshell-parse-argument-hook
 When each function on this hook is called, point will be at the
 current position within the argument list.  The function should either
 return nil, meaning that it did no argument parsing, or it should
-return the result of the parse as a sexp.  It is also responsible for
-moving the point forward to reflect the amount of input text that was
-parsed.
+return the result of the parse as a sexp.  If the function did do
+argument parsing, but the result was nothing at all, it should return
+`eshell-empty-token'.  The function is also responsible for moving the
+point forward to reflect the amount of input text that was parsed.
 
 If the hook determines that it has reached the end of an argument, it
 should call `eshell-finish-arg' to complete processing of the current
@@ -325,13 +326,14 @@ eshell-parse-argument
 		   (prog1
 		       (char-to-string (char-after))
 		     (forward-char)))))
-	  (if (not eshell-current-argument)
-	      (setq eshell-current-argument result)
-	    (unless eshell-arg-listified
-	      (setq eshell-current-argument
-		    (list eshell-current-argument)
-		    eshell-arg-listified t))
-	    (nconc eshell-current-argument (list result))))))
+          (unless (eq result 'eshell-empty-token)
+            (if (not eshell-current-argument)
+                (setq eshell-current-argument result)
+              (unless eshell-arg-listified
+                (setq eshell-current-argument
+                      (list eshell-current-argument)
+                      eshell-arg-listified t))
+              (nconc eshell-current-argument (list result)))))))
     (when (and outer eshell-current-argument)
       (add-text-properties arg-begin (1+ arg-begin)
 			   '(arg-begin t rear-nonsticky
@@ -373,17 +375,25 @@ eshell-parse-backslash
 after are both returned."
   (when (eq (char-after) ?\\)
     (when (eshell-looking-at-backslash-return (point))
-	(throw 'eshell-incomplete ?\\))
+        (throw 'eshell-incomplete ?\\))
     (forward-char 2) ; Move one char past the backslash.
     ;; If the char is in a quote, backslash only has special meaning
     ;; if it is escaping a special char.
-    (if eshell-current-quoted
-        (if (memq (char-before) eshell-special-chars-inside-quoting)
-            (list 'eshell-escape-arg (char-to-string (char-before)))
-          (concat "\\" (char-to-string (char-before))))
-      (if (memq (char-before) eshell-special-chars-outside-quoting)
-          (list 'eshell-escape-arg (char-to-string (char-before)))
-        (char-to-string (char-before))))))
+    (let ((special-chars (if eshell-current-quoted
+                             eshell-special-chars-inside-quoting
+                           eshell-special-chars-outside-quoting)))
+      (cond
+       ;; Escaped newlines are extra-special: they expand to an empty
+       ;; token to allow for continuing Eshell commands across
+       ;; multiple lines.
+       ((eq (char-before) ?\n)
+        'eshell-empty-token)
+       ((memq (char-before) special-chars)
+        (list 'eshell-escape-arg (char-to-string (char-before))))
+       (eshell-current-quoted
+        (concat "\\" (char-to-string (char-before))))
+       (t
+        (char-to-string (char-before)))))))
 
 (defun eshell-parse-literal-quote ()
   "Parse a literally quoted string.  Nothing has special meaning!"
diff --git a/test/lisp/eshell/esh-arg-tests.el b/test/lisp/eshell/esh-arg-tests.el
new file mode 100644
index 0000000000..77f9404d4c
--- /dev/null
+++ b/test/lisp/eshell/esh-arg-tests.el
@@ -0,0 +1,105 @@
+;;; esh-arg-tests.el --- esh-arg test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's argument handling.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+(defvar eshell-test-value nil)
+
+;;; Tests:
+
+(ert-deftest esh-arg-test/escape/nonspecial ()
+  "Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
+special character."
+  (with-temp-eshell
+   (eshell-match-command-output "echo he\\llo"
+                                "hello\n")))
+
+(ert-deftest esh-arg-test/escape/nonspecial-unicode ()
+  "Test that \"\\c\" and \"c\" are equivalent when \"c\" is a
+unicode character (unicode characters are nonspecial by
+definition)."
+  (with-temp-eshell
+   (eshell-match-command-output "echo Vid\\éos"
+                                "Vidéos\n")))
+
+(ert-deftest esh-arg-test/escape/special ()
+  "Test that the backslash is not preserved for escaped special
+chars."
+  (with-temp-eshell
+   (eshell-match-command-output "echo he\\\\llo"
+                                ;; Backslashes are doubled for regexp.
+                                "he\\\\llo\n")))
+
+(ert-deftest esh-arg-test/escape/newline ()
+  "Test that an escaped newline is equivalent to the empty string.
+When newlines are *nonspecial*, an escaped newline should be
+treated as just a newline."
+  (with-temp-eshell
+   (eshell-match-command-output "echo hi\\\nthere"
+                                "hithere\n")))
+
+(ert-deftest esh-arg-test/escape/newline-conditional ()
+  "Test invocation of an if/else statement using line continuations."
+  (let ((eshell-test-value t))
+    (eshell-command-result-equal
+     "if $eshell-test-value \\\n{echo yes} \\\n{echo no}"
+     "yes"))
+  (let ((eshell-test-value nil))
+    (eshell-command-result-equal
+     "if $eshell-test-value \\\n{echo yes} \\\n{echo no}"
+     "no")))
+
+(ert-deftest esh-arg-test/escape-quoted/nonspecial ()
+  "Test that the backslash is preserved for escaped nonspecial
+chars."
+  (with-temp-eshell
+   (eshell-match-command-output "echo \"h\\i\""
+                                ;; Backslashes are doubled for regexp.
+                                "h\\\\i\n")))
+
+(ert-deftest esh-arg-test/escape-quoted/special ()
+  "Test that the backslash is not preserved for escaped special
+chars."
+  (with-temp-eshell
+   (eshell-match-command-output "echo \"\\\"hi\\\\\""
+                                ;; Backslashes are doubled for regexp.
+                                "\\\"hi\\\\\n")))
+
+(ert-deftest esh-arg-test/escape-quoted/newline ()
+  "Test that an escaped newline is equivalent to the empty string.
+When newlines are *nonspecial*, an escaped newline should be
+treated literally, as a backslash and a newline."
+  (with-temp-eshell
+   (eshell-match-command-output "echo \"hi\\\nthere\""
+                                "hithere\n")))
+
+;; esh-arg-tests.el ends here
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index d5112146c2..c67ac67fd3 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -105,37 +105,6 @@ eshell-test/lisp-reset-in-pipeline
      (format template "format \"%s\" eshell-in-pipeline-p")
      "nil")))
 
-(ert-deftest eshell-test/escape-nonspecial ()
-  "Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
-special character."
-  (with-temp-eshell
-   (eshell-match-command-output "echo he\\llo"
-                                "hello\n")))
-
-(ert-deftest eshell-test/escape-nonspecial-unicode ()
-  "Test that \"\\c\" and \"c\" are equivalent when \"c\" is a
-unicode character (unicode characters are nonspecial by
-definition)."
-  (with-temp-eshell
-   (eshell-match-command-output "echo Vid\\éos"
-                                "Vidéos\n")))
-
-(ert-deftest eshell-test/escape-nonspecial-quoted ()
-  "Test that the backslash is preserved for escaped nonspecial
-chars"
-  (with-temp-eshell
-   (eshell-match-command-output "echo \"h\\i\""
-                                ;; Backslashes are doubled for regexp.
-                                "h\\\\i\n")))
-
-(ert-deftest eshell-test/escape-special-quoted ()
-  "Test that the backslash is not preserved for escaped special
-chars"
-  (with-temp-eshell
-   (eshell-match-command-output "echo \"\\\"hi\\\\\""
-                                ;; Backslashes are doubled for regexp.
-                                "\\\"hi\\\\\n")))
-
 (ert-deftest eshell-test/command-running-p ()
   "Modeline should show no command running"
   (with-temp-eshell
-- 
2.25.1


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

end of thread, other threads:[~2022-12-09  0:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-27  0:36 bug#59622: 29.0.50; [PATCH] Regression in Eshell's handling of escaped newlines Jim Porter
2022-11-27  0:42 ` Jim Porter
2022-12-04  1:41 ` Jim Porter
2022-12-04  7:26   ` Eli Zaretskii
2022-12-05  1:35     ` Jim Porter
2022-12-05 12:39       ` Eli Zaretskii
2022-12-07  4:18         ` Jim Porter
2022-12-07 13:34           ` Eli Zaretskii
2022-12-07 17:57             ` Jim Porter
2022-12-07 18:10               ` Eli Zaretskii
2022-12-08  5:47                 ` Jim Porter
2022-12-09  0:59                   ` Jim Porter

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.