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

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

[J.P.]

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

Tags: patch

This apparently began after

  2f181d60323bd9e0196775828de633100479f4c2
  Author:     Stefan Monnier <monnier@iro.umontreal.ca>
  AuthorDate: Fri Jun 16 13:35:06 2023 -0400
  CommitDate: Sat Jun 17 17:24:38 2023 -0400

  pp.el (pp-fill): New default pp function
  
  1 file changed, 90 insertions(+), 1 deletion(-)
  lisp/emacs-lisp/pp.el | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++-

So I guess it also affects Emacs 30. To reproduce, put this in
test/lisp/emacs-lisp/ert-tests.el:

   (ert-deftest ert--pp-with-indentation-and-newline ()
     (should (equal '((:one "1" :three "3" :two "2")) '((:one "1")))))

Then run something like:

   $ make -C test \
     SELECTOR=ert--pp-with-indentation-and-newline \
     lisp/emacs-lisp/ert-tests.log

   Error: scan-error ("Containing expression ends prematurely" 221 221)
     forward-sexp-default-function(-1)
     forward-sexp(-1)
     calculate-lisp-indent((5 221 238 nil nil nil 4 nil nil (5 27 161 175
     lisp-indent-calc-next(#s(lisp-indent-state :stack (50 16 6 5 nil) :p
     indent-sexp()
     ert--pp-with-indentation-and-newline((ert-test-failed ((should (equa
     #f(compiled-function (event-type &rest event-args) #<bytecode 0x16b6
     #f(compiled-function () #<bytecode -0xb4ce749a56118cd>)()
     ert-run-or-rerun-test(#s(ert--stats :selector ert--pp-with-indentati
   
   Aborted: Ran 1 tests, 0 results as expected, 1 unexpected, -1 skipped
   
No idea if there's a deeper issue at play here, maybe something in
pp.el that a patch like the attached would just be papering over.


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.43, cairo version 1.18.0) of 2024-08-09 built on localhost
Repository revision: 944e45db53cb173c5eadd4794081c133e8649d67
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12401002
System Description: Fedora Linux 40 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
bytecomp byte-compile comp-common rx emacsbug message mailcap yank-media
puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo gtk
x-toolkit xinput2 x multi-tty move-toolbar make-network-process
native-compile emacs)

Memory information:
((conses 16 58857 9185) (symbols 48 6741 0) (strings 32 16558 3964)
 (string-bytes 1 477530) (vectors 16 11257)
 (vector-slots 8 135809 8706) (floats 8 21 4) (intervals 56 249 0)
 (buffers 984 11))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Indent-failed-ERT-should-forms-with-Emacs-Lisp-synta.patch --]
[-- Type: text/x-patch, Size: 3790 bytes --]

From 2a65e655ae62c1ee74a15747c228b3c19a871f9e Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 9 Aug 2024 16:49:28 -0700
Subject: [PATCH] Indent failed ERT should forms with Emacs Lisp syntax

* lisp/emacs-lisp/ert.el (ert--pp-with-indentation-and-newline):
Use `emacs-lisp-mode-syntax-table' when indenting.
* test/lisp/emacs-lisp/ert-tests.el
(ert--pp-with-indentation-and-newline): New test.
---
 lisp/emacs-lisp/ert.el            |  3 +-
 test/lisp/emacs-lisp/ert-tests.el | 54 +++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 6a665c8181d..6f5a0093ba0 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -1323,7 +1323,8 @@ ert--pp-with-indentation-and-newline
     (unless (bolp) (insert "\n"))
     (save-excursion
       (goto-char begin)
-      (indent-sexp))))
+      (with-syntax-table emacs-lisp-mode-syntax-table
+        (indent-sexp)))))
 
 (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

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

[Eli Zaretskii]

> From: "J.P." <jp@neverwas.me>
> Date: Sat, 10 Aug 2024 06:48:11 -0700
> 
> Tags: patch
> 
> This apparently began after
> 
>   2f181d60323bd9e0196775828de633100479f4c2
>   Author:     Stefan Monnier <monnier@iro.umontreal.ca>
>   AuthorDate: Fri Jun 16 13:35:06 2023 -0400
>   CommitDate: Sat Jun 17 17:24:38 2023 -0400
> 
>   pp.el (pp-fill): New default pp function
>   
>   1 file changed, 90 insertions(+), 1 deletion(-)
>   lisp/emacs-lisp/pp.el | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 
> So I guess it also affects Emacs 30. To reproduce, put this in
> test/lisp/emacs-lisp/ert-tests.el:
> 
>    (ert-deftest ert--pp-with-indentation-and-newline ()
>      (should (equal '((:one "1" :three "3" :two "2")) '((:one "1")))))
> 
> Then run something like:
> 
>    $ make -C test \
>      SELECTOR=ert--pp-with-indentation-and-newline \
>      lisp/emacs-lisp/ert-tests.log
> 
>    Error: scan-error ("Containing expression ends prematurely" 221 221)
>      forward-sexp-default-function(-1)
>      forward-sexp(-1)
>      calculate-lisp-indent((5 221 238 nil nil nil 4 nil nil (5 27 161 175
>      lisp-indent-calc-next(#s(lisp-indent-state :stack (50 16 6 5 nil) :p
>      indent-sexp()
>      ert--pp-with-indentation-and-newline((ert-test-failed ((should (equa
>      #f(compiled-function (event-type &rest event-args) #<bytecode 0x16b6
>      #f(compiled-function () #<bytecode -0xb4ce749a56118cd>)()
>      ert-run-or-rerun-test(#s(ert--stats :selector ert--pp-with-indentati
>    
>    Aborted: Ran 1 tests, 0 results as expected, 1 unexpected, -1 skipped
>    
> No idea if there's a deeper issue at play here, maybe something in
> pp.el that a patch like the attached would just be papering over.

Adding Stefan.

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>    Error: scan-error ("Containing expression ends prematurely" 221 221)
>      forward-sexp-default-function(-1)
>      forward-sexp(-1)
>      calculate-lisp-indent((5 221 238 nil nil nil 4 nil nil (5 27 161 175
>      lisp-indent-calc-next(#s(lisp-indent-state :stack (50 16 6 5 nil) :p
>      indent-sexp()
>      ert--pp-with-indentation-and-newline((ert-test-failed ((should (equa

Hmm...

> --- a/lisp/emacs-lisp/ert.el
> +++ b/lisp/emacs-lisp/ert.el
> @@ -1323,7 +1323,8 @@ ert--pp-with-indentation-and-newline
>      (unless (bolp) (insert "\n"))
>      (save-excursion
>        (goto-char begin)
> -      (indent-sexp))))
> +      (with-syntax-table emacs-lisp-mode-syntax-table
> +        (indent-sexp)))))

Your patch makes sense: indeed, looking at the code of `indent-sexp`,
I see that it uses `lisp-indent*` functions in a way which presumes that
we're looking at Lisp code and would require a list-mode syntax-table.
I wonder why this has not bitten us earlier in other circumstances.

But I also wonder why `ert--pp-with-indentation-and-newline` calls
`indent-sexp`, since `pp` should have done that for us already, so I'd
be tempted to just remove that call.  Or maybe the purpose is to "shift"
the text when `begin` is not in column 0?
If so, maybe `indent-rigidly` is a better way to get the same result?


        Stefan

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

[J.P.]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Your patch makes sense: indeed, looking at the code of `indent-sexp`,
> I see that it uses `lisp-indent*` functions in a way which presumes that
> we're looking at Lisp code and would require a list-mode syntax-table.
> I wonder why this has not bitten us earlier in other circumstances.
>
> But I also wonder why `ert--pp-with-indentation-and-newline` calls
> `indent-sexp`, since `pp` should have done that for us already, so I'd
> be tempted to just remove that call.  Or maybe the purpose is to "shift"
> the text when `begin` is not in column 0?
> If so, maybe `indent-rigidly` is a better way to get the same result?

Right, I don't think `begin' is ever in column 0 (as currently used). So
I guess the intention is indeed to shift all but the first line of the
`pp' result by `current-column', meaning it's probably cleaner (as you
say) to do a dumb, uniform shift.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Indent-ERT-failure-explanations-rigidly.patch --]
[-- Type: text/x-patch, Size: 3877 bytes --]

From ee67372f30150efef18284b0465f535891e68da9 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 9 Aug 2024 16:49:28 -0700
Subject: [PATCH] Indent ERT failure explanations rigidly

* lisp/emacs-lisp/ert.el (ert--pp-with-indentation-and-newline):
Indent uniformly from the line after point to the end of the
pretty-printed result.
* test/lisp/emacs-lisp/ert-tests.el
(ert--pp-with-indentation-and-newline): New test.  (Bug#72561)
---
 lisp/emacs-lisp/ert.el            |  4 ++-
 test/lisp/emacs-lisp/ert-tests.el | 54 +++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/ert.el b/lisp/emacs-lisp/ert.el
index 2d96e5ce5a9..a1c70aa1144 100644
--- a/lisp/emacs-lisp/ert.el
+++ b/lisp/emacs-lisp/ert.el
@@ -1323,7 +1323,9 @@ ert--pp-with-indentation-and-newline
     (unless (bolp) (insert "\n"))
     (save-excursion
       (goto-char begin)
-      (indent-sexp))))
+      (let ((cols (- (point) (line-beginning-position))))
+        (forward-line)
+        (indent-rigidly begin (point-max) 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

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

[J.P.]

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

"J.P." <jp@neverwas.me> writes:

Actually, that can probably be simplified a bit.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Indent-ERT-failure-explanations-rigidly.patch --]
[-- Type: text/x-patch, Size: 4091 bytes --]

From e240ba51f780c84e57d3ba34ada83489149d371a Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Fri, 9 Aug 2024 16:49:28 -0700
Subject: [PATCH] Indent ERT failure explanations rigidly

* lisp/emacs-lisp/ert.el (ert--pp-with-indentation-and-newline):
Indent uniformly from the line after point to the end of the
pretty-printed result.
* 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..d4cc43d730d 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 (- (point) (line-beginning-position)))
         (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)))
 
 (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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> 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)

>          (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.


        Stefan

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

[J.P.]

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

Stefan Monnier <monnier@iro.umontreal.ca> 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)?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Indent-ERT-failure-explanations-rigidly.patch --]
[-- Type: text/x-patch, Size: 4194 bytes --]

From 99196569034bb6447f8d583b19f53e21e51ade56 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
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

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

[Eli Zaretskii]

> From: "J.P." <jp@neverwas.me>
> Cc: 72561@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 22 Aug 2024 18:02:20 -0700
> 
> And since the issue is new on the release branch, I'm guessing that's
> where a patch should go (Cc. Eli)?

If you run the entire test suite, including the expensive tests, and
this doesn't change the results, then yes, please install on the
release branch.

Thanks.

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

[J.P.]

Eli Zaretskii <eliz@gnu.org> writes:

> If you run the entire test suite, including the expensive tests, and
> this doesn't change the results, then yes, please install on the
> release branch.

Results for check-expensive were stable, running locally in EMBA's
inotify and native-comp containers. The changes were therefore installed
on the release branch. Thanks and closing.