bug#58810: 29.0.50; [PATCH] 'eshell-stringify' sometimes produces wrong results

bug#58810: 29.0.50; [PATCH] 'eshell-stringify' sometimes produces wrong results

[Jim Porter]

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

Starting from "emacs -Q -f -eshell":

   ~ $ echo $(list 'quote 'hello)
   'hell

The last character of "hello" was stripped. That's because 
'eshell-stringify' tries to remove the trailing newline from 
'pp-to-string', but it does so by always stripping the last character 
from the result. Obviously, that only works when the last character 
really is a newline.

Attached is a patch with regression tests for 'eshell-stringify' so that 
this works correctly.

[-- Attachment #2: 0001-Only-strip-newlines-when-stringifying-a-value-for-Es.patch --]
[-- Type: text/plain, Size: 3902 bytes --]

From bb440316a5a383bab091e5b355778db502ba1a3b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 18 Sep 2022 17:42:09 -0700
Subject: [PATCH] Only strip newlines when stringifying a value for Eshell

* lisp/eshell/esh-util.el (eshell-stringify): Use 'string-trim-right'
instead of stripping the last character of the result of
'pp-to-string'.

* test/lisp/eshell/esh-util-tests.el: New file.
---
 lisp/eshell/esh-util.el            | 11 +++---
 test/lisp/eshell/esh-util-tests.el | 57 ++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 7 deletions(-)
 create mode 100644 test/lisp/eshell/esh-util-tests.el

diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index f47373c115..ecb6888651 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -350,16 +350,13 @@ eshell-stringify
   "Convert OBJECT into a string value."
   (cond
    ((stringp object) object)
-   ((and (listp object)
-	 (not (eq object nil)))
-    (let ((string (pp-to-string object)))
-      (substring string 0 (1- (length string)))))
    ((numberp object)
     (number-to-string object))
+   ((and (eq object t)
+	 (not eshell-stringify-t))
+    nil)
    (t
-    (unless (and (eq object t)
-		 (not eshell-stringify-t))
-      (pp-to-string object)))))
+    (string-trim-right (pp-to-string object)))))
 
 (defsubst eshell-stringify-list (args)
   "Convert each element of ARGS into a string value."
diff --git a/test/lisp/eshell/esh-util-tests.el b/test/lisp/eshell/esh-util-tests.el
new file mode 100644
index 0000000000..1cbd015999
--- /dev/null
+++ b/test/lisp/eshell/esh-util-tests.el
@@ -0,0 +1,57 @@
+;;; esh-util-tests.el --- esh-util 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/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-util)
+
+;;; Tests:
+
+(ert-deftest esh-util-test/eshell-stringify/string ()
+  "Test that `eshell-stringify' preserves the value of strings."
+  (should (equal (eshell-stringify "hello") "hello")))
+
+(ert-deftest esh-util-test/eshell-stringify/number ()
+  "Test that `eshell-stringify' converts numbers to strings."
+  (should (equal (eshell-stringify 42) "42"))
+  (should (equal (eshell-stringify 4.2) "4.2")))
+
+(ert-deftest esh-util-test/eshell-stringify/t ()
+  "Test that `eshell-stringify' treats `t' according to `eshell-stringify-t'."
+  (let ((eshell-stringify-t t))
+    (should (equal (eshell-stringify t) "t")))
+  (let ((eshell-stringify-t nil))
+    (should (equal (eshell-stringify t) nil))))
+
+(ert-deftest esh-util-test/eshell-stringify/nil ()
+  "Test that `eshell-stringify' converts nil to a string."
+  (should (equal (eshell-stringify nil) "nil")))
+
+(ert-deftest esh-util-test/eshell-stringify/list ()
+  "Test that `eshell-stringify' correctly stringifies lists."
+  (should (equal (eshell-stringify '(1 2 3)) "(1 2 3)"))
+  (should (equal (eshell-stringify '((1 2) (3 . 4)))
+                 "((1 2)\n (3 . 4))")))
+
+(ert-deftest esh-util-test/eshell-stringify/complex ()
+  "Test that `eshell-stringify' correctly stringifies complex objects."
+  (should (equal (eshell-stringify (list 'quote 'hello)) "'hello")))
+
+;;; esh-util-tests.el ends here
-- 
2.25.1

bug#58810: 29.0.50; [PATCH] 'eshell-stringify' sometimes produces wrong results

[Jim Porter]

On 10/26/2022 6:22 PM, Jim Porter wrote:
> Attached is a patch with regression tests for 'eshell-stringify' so that 
> this works correctly.

Merged as d2a9dae40057ff16683d9c5f30a3b04500ebc4cf. Closing this bug.