Subject: bug#72561: 31.0.50; Scan error in ert--pp-with-indentation-and-newline

From: "J.P." <[email-masked]>
Date: Thu, 22 Aug 2024 18:02:20 -0700

Stefan Monnier <[email-masked]> writes:

>> Actually, that can probably be simplified a bit.
>
> BTW, it could be argued that the `indent-rigidly` should take place in
> `pp` (i.e. consider it as a bug in `pp`).
>
> The patch looks good to me, tho I have the following nitpicks:
>
>>    (let ((begin (point))
>> +        (cols (- (point) (line-beginning-position)))
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                  (current-column)

Oh, TABs and stuff, right.

>
>>          (pp-escape-newlines t)
>>          (print-escape-control-characters t))
>>      (pp object (current-buffer))
>>      (unless (bolp) (insert "\n"))
>> -    (save-excursion
>> -      (goto-char begin)
>> -      (indent-sexp))))
>> +    (indent-rigidly begin (point-max) cols)))
>>                            ^^^^^^^^^^^
>                              (point)
>
> We arguably know that (point) is the same as (point-max) here, so it's
> not really important, but (point) is shorter and conceptually more
> correct since we wouldn't want to shift text that was there before.

Makes sense. Might as well have the function be more generally useful.
Thanks.

And since the issue is new on the release branch, I'm guessing that's
where a patch should go (Cc. Eli)?

>From 99196569034bb6447f8d583b19f53e21e51ade56 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email-masked]>
Date: Fri, 9 Aug 2024 16:49:28 -0700
Subject: [PATCH] Indent ERT failure explanations rigidly

This also affects the listing of `should' forms produced by hitting
the L key on a test button in an ERT buffer.

* lisp/emacs-lisp/ert.el (ert--pp-with-indentation-and-newline):
Indent the pretty-printed result to match the caller's current column
as a reference indentation.
* test/lisp/emacs-lisp/ert-tests.el
(ert--pp-with-indentation-and-newline): New test.  (Bug#72561)
---
 lisp/emacs-lisp/ert.el            |  5 ++-
 test/lisp/emacs-lisp/ert-tests.el | 54 +++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 2d96e5ce5a9..105c44d49aa 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -1317,13 +1317,12 @@ ert--pp-with-indentation-and-newline
   "Pretty-print OBJECT, indenting it to the current column of point.
 Ensures a final newline is inserted."
   (let ((begin (point))
+        (cols (current-column))
         (pp-escape-newlines t)
         (print-escape-control-characters t))
     (pp object (current-buffer))
     (unless (bolp) (insert "\n"))
-    (save-excursion
-      (goto-char begin)
-      (indent-sexp))))
+    (indent-rigidly begin (point) cols)))
 
 (defun ert--insert-infos (result)
   "Insert `ert-info' infos from RESULT into current buffer.
diff --git a/test/lisp/emacs-lisp/ert-tests.el b/test/lisp/emacs-lisp/ert-tests.el
index 1aff73d66f6..cdbeae2f2e5 100644
--- a/test/lisp/emacs-lisp/ert-tests.el
+++ b/test/lisp/emacs-lisp/ert-tests.el
@@ -876,6 +876,60 @@ ert-test-get-explainer
   (should (eq (ert--get-explainer 'string-equal) 'ert--explain-string-equal))
   (should (eq (ert--get-explainer 'string=) 'ert--explain-string-equal)))
 
+(ert-deftest ert--pp-with-indentation-and-newline ()
+  :tags '(:causes-redisplay)
+  (let ((failing-test (make-ert-test
+                       :name 'failing-test
+                       :body (lambda ()
+                               (should (equal '((:one "1" :three "3" :two "2"))
+                                              '((:one "1")))))))
+        (want-body "\
+Selector: <failing-test>
+Passed:  0
+Failed:  1 (1 unexpected)
+Skipped: 0
+Total:   1/1
+
+Started at:   @@TIMESTAMP@@
+Finished.
+Finished at:  @@TIMESTAMP@@
+
+F
+
+F failing-test
+    (ert-test-failed
+     ((should (equal '((:one \"1\" :three \"3\" :two \"2\")) '((:one \"1\"))))
+      :form (equal ((:one \"1\" :three \"3\" :two \"2\")) ((:one \"1\"))) :value
+      nil :explanation
+      (list-elt 0
+                (proper-lists-of-different-length 6 2
+                                                  (:one \"1\" :three \"3\"
+                                                        :two \"2\")
+                                                  (:one \"1\")
+                                                  first-mismatch-at 2))))
+\n\n")
+        (want-msg "Ran 1 tests, 0 results were as expected, 1 unexpected")
+        (buffer-name (generate-new-buffer-name " *ert-test-run-tests*")))
+    (cl-letf* ((ert-debug-on-error nil)
+               (ert--output-buffer-name buffer-name)
+               (messages nil)
+               ((symbol-function 'message)
+                (lambda (format-string &rest args)
+                  (push (apply #'format format-string args) messages)))
+               ((symbol-function 'ert--format-time-iso8601)
+                (lambda (_) "@@TIMESTAMP@@")))
+      (save-window-excursion
+        (unwind-protect
+            (let ((case-fold-search nil))
+              (ert-run-tests-interactively failing-test)
+              (should (equal (list want-msg) messages))
+              (should (equal (string-replace "\t" "        "
+                                             (with-current-buffer buffer-name
+                                               (buffer-string)))
+                             want-body)))
+          (when noninteractive
+            (kill-buffer buffer-name)))))))
+
 (provide 'ert-tests)
 
 ;;; ert-tests.el ends here
-- 
2.46.0