Subject: Re: bug#8531: 24.0.50;

From: Samer Masterson <[email-masked]>
Date: Sun, 15 Mar 2015 04:58:58 -0700

Hi,

This is the updated patch. Thanks in advance for the review.

Just to clarify, the bug is that

    $ ls Vid\éos

doesn't do the right thing in eshell (it preserves the backslash if
the character after it is non-special). This diverges from POSIX and
Bash.

New behavior:

Outside of a double quoted string, the backslash escapes the following
charater if that character is special. Otherwise it simply returns the
character afterwards (the backslash is ignored).

I expanded `eshell-parse-backslash's behavior to match Bash for quoted
strings as well. Inside a double quoted string, the backslash escapes
the following character if that character is special, otherwise the
backslash is preserved.

Best,
Samer

Patch below:
[PATCH] Fix bug#8531

* eshell/esh-arg.el (eshell-parse-argument-hook): Update comment.
(eshell-parse-backslash): Return escaped character after backslash
if it is special.  Otherwise, if the backslash is not in a quoted
string, ignore the backslash and return the character after; if
the backslash is in a quoted string, return the backslash and the
character after.  (bug#8531)
* automated/eshell.el (eshell-test/escape-nonspecial)
(eshell-test/escape-nonspecial-unicode)
(eshell-test/escape-nonspecial-quoted)
(eshell-test/escape-special-quoted): Add tests for new
`eshell-parse-backslash' behavior.  (bug#8531)
---
 lisp/ChangeLog           |  9 +++++++++
 lisp/eshell/esh-arg.el   | 49 +++++++++++++++++++++---------------------------
 test/ChangeLog           |  8 ++++++++
 test/automated/eshell.el | 31 ++++++++++++++++++++++++++++++
 4 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index a9cf1b0..209382d 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,12 @@
+2015-03-15  Samer Masterson  <[email-masked]>
+
+	* eshell/esh-arg.el (eshell-parse-argument-hook): Update comment.
+	(eshell-parse-backslash): Return escaped character after backslash
+	if it is special.  Otherwise, if the backslash is not in a quoted
+	string, ignore the backslash and return the character after; if
+	the backslash is in a quoted string, return the backslash and the
+	character after.  (bug#8531)
+
 2015-03-14  Michael R. Mauger  <[email-masked]>
 
 	* progmodes/sql.el: Version 3.5
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 5c7d7ca..a5f697f 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -89,7 +89,7 @@ yield the values intended."
 	  (goto-char (match-end 0))
 	  (eshell-finish-arg)))))
 
-   ;; backslash before a special character means escape it
+   ;; parse backslash and the character after
    'eshell-parse-backslash
 
    ;; text beginning with ' is a literally quoted
@@ -305,34 +305,27 @@ If the character is itself a backslash, it needs no escaping."
 	  (string ?\\ char)))))
 
 (defun eshell-parse-backslash ()
-  "Parse a single backslash (\) character, which might mean escape.
-It only means escape if the character immediately following is a
-special character that is not itself a backslash."
+  "Parse a single backslash (\\) character and the character after.
+If the character after the backslash is special, always ignore
+the backslash and return the escaped character.
+
+Otherwise, if the backslash is not in quoted string, the
+backslash is ignored and the character after is returned.  If the
+backslash is in a quoted string, the backslash and the character
+after are both returned."
   (when (eq (char-after) ?\\)
-    (if (eshell-looking-at-backslash-return (point))
-	(throw 'eshell-incomplete ?\\)
-      (if (and (not (eq (char-after (1+ (point))) ?\\))
-	       (if eshell-current-quoted
-		   (memq (char-after (1+ (point)))
-			 eshell-special-chars-inside-quoting)
-		 (memq (char-after (1+ (point)))
-		       eshell-special-chars-outside-quoting)))
-	  (progn
-	    (forward-char 2)
-	    (list 'eshell-escape-arg
-		  (char-to-string (char-before))))
-	;; allow \\<RET> to mean a literal "\" character followed by a
-	;; normal return, rather than a backslash followed by a line
-	;; continuation (i.e., "\\ + \n" rather than "\ + \\n").  This
-	;; is necessary because backslashes in Eshell are not special
-	;; unless they either precede something special, or precede a
-	;; backslash that precedes something special.  (Mainly this is
-	;; done to make using backslash on Windows systems more
-	;; natural-feeling).
-	(if (eshell-looking-at-backslash-return (1+ (point)))
-	    (forward-char))
-	(forward-char)
-	"\\"))))
+    (when (eshell-looking-at-backslash-return (point))
+	(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))))))
 
 (defun eshell-parse-literal-quote ()
   "Parse a literally quoted string.  Nothing has special meaning!"
diff --git a/test/ChangeLog b/test/ChangeLog
index 6a474e1..ff2cd60 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-15  Samer Masterson  <[email-masked]>
+
+	* automated/eshell.el (eshell-test/escape-nonspecial)
+	(eshell-test/escape-nonspecial-unicode)
+	(eshell-test/escape-nonspecial-quoted)
+	(eshell-test/escape-special-quoted): Add tests for new
+	`eshell-parse-backslash' behavior.  (bug#8531)
+
 2015-03-10  Jackson Ray Hamilton  <[email-masked]>
 
 	* indent/js-indent-init-dynamic.js: Fix spelling error.
diff --git a/test/automated/eshell.el b/test/automated/eshell.el
index d51355f..81898db 100644
--- a/test/automated/eshell.el
+++ b/test/automated/eshell.el
@@ -166,6 +166,37 @@ e.g. \"{(+ 1 2)} 3\" => 3"
    (eshell-command-result-p "+ 1 2; + $_ 4"
                              "3\n6\n")))
 
+(ert-deftest eshell-test/escape-nonspecial ()
+  "Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
+special character."
+  (with-temp-eshell
+   (eshell-command-result-p "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-command-result-p "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-command-result-p "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-command-result-p "echo \"h\\\\i\""
+                            ;; Backslashes are doubled for regexp.
+                            "h\\\\i\n")))
+
 (ert-deftest eshell-test/command-running-p ()
   "Modeline should show no command running"
   (with-temp-eshell