bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

[No Wayman]

Observe the output of the following two expressions:


(eval (read (with-temp-buffer
              (pp-emacs-lisp-code '`(1+ ,(1+ 1)))
              (buffer-string)))
      t)

;; => eval: Invalid function: `(1+ (,(1+ 1)))

(eval
 (read (with-temp-buffer
         (insert (pp-to-string '`(1+ ,(1+ 1))))
         (buffer-string)))
 t)

;; => (1+ 2)


The form returned by pp-emacs-lisp-code is:

(` (1+ (, (1+ 1))))

The backqoute and comma are not escaped, resulting in the error.
It looks like the logic in pp--insert-lisp would need to be 
extended to account for backquoting constructs.

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-02-03 built on laptop
Repository revision: 4ebded3f5ee8617ac6b1debaa01706cd78206f39
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 
11.0.12101011
System Description: Arch Linux

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

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

No Wayman <iarchivedmywholelife@gmail.com> writes:

> The backqoute and comma are not escaped, resulting in the error.
> It looks like the logic in pp--insert-lisp would need to be extended
> to account for backquoting constructs.

Indeed - the function formats the expression as a list.

The second problem is that the recursive calls of `pp--insert-lisp'
print symbols using `princ' (i.e., without quoting), so even the list
representation is wrong.

Michael.

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

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

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> The second problem is that the recursive calls of `pp--insert-lisp'
> print symbols using `princ' (i.e., without quoting), so even the list
> representation is wrong.

I think we should print any symbol using `prin1', not `princ' - printing
symbols that need quoting without quoting is not useful:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-pp-emacs-lisp-code-printing-of-symbols.patch --]
[-- Type: text/x-diff, Size: 820 bytes --]

From f493708a456608f59b29c3f2308ecc51177667ec Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Sun, 18 Feb 2024 02:48:15 +0100
Subject: [PATCH] Fix pp-emacs-lisp-code printing of symbols

* lisp/emacs-lisp/pp.el (pp--insert-lisp): Print symbols
readably (bug#69168).
---
 lisp/emacs-lisp/pp.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/emacs-lisp/pp.el b/lisp/emacs-lisp/pp.el
index 1d722051406..944dd750839 100644
--- a/lisp/emacs-lisp/pp.el
+++ b/lisp/emacs-lisp/pp.el
@@ -458,6 +458,8 @@ pp--insert-lisp
     (string
      (let ((print-escape-newlines t))
        (prin1 sexp (current-buffer))))
+    (symbol
+     (prin1 sexp (current-buffer)))
     (otherwise (princ sexp (current-buffer)))))

 (defun pp--format-vector (sexp)
--
2.39.2


[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Michael.

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

[Eli Zaretskii]

> Cc: 69168@debbugs.gnu.org
> Date: Sun, 25 Feb 2024 06:00:38 +0100
> From:  Michael Heerdegen via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> Michael Heerdegen <michael_heerdegen@web.de> writes:
> 
> > The second problem is that the recursive calls of `pp--insert-lisp'
> > print symbols using `princ' (i.e., without quoting), so even the list
> > representation is wrong.
> 
> I think we should print any symbol using `prin1', not `princ' - printing
> symbols that need quoting without quoting is not useful:
> 
> 
> From f493708a456608f59b29c3f2308ecc51177667ec Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Sun, 18 Feb 2024 02:48:15 +0100
> Subject: [PATCH] Fix pp-emacs-lisp-code printing of symbols
> 
> * lisp/emacs-lisp/pp.el (pp--insert-lisp): Print symbols
> readably (bug#69168).
> ---
>  lisp/emacs-lisp/pp.el | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lisp/emacs-lisp/pp.el b/lisp/emacs-lisp/pp.el
> index 1d722051406..944dd750839 100644
> --- a/lisp/emacs-lisp/pp.el
> +++ b/lisp/emacs-lisp/pp.el
> @@ -458,6 +458,8 @@ pp--insert-lisp
>      (string
>       (let ((print-escape-newlines t))
>         (prin1 sexp (current-buffer))))
> +    (symbol
> +     (prin1 sexp (current-buffer)))
>      (otherwise (princ sexp (current-buffer)))))
> 
>  (defun pp--format-vector (sexp)
> --
> 2.39.2

Thanks.  Stefan, any comments?

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

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

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  Stefan, any comments?

One from me: I'm working on a second patch for pp that will make
printing of backquote forms using the nicer `X syntax.  But this small
fix is independent of such improvements.

Michael.

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

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

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> One from me: I'm working on a second patch for pp that will make
> printing of backquote forms using the nicer `X syntax.

Here is the additional patch.  No Wayman, maybe you can try if it gives
good results?

This change seems quite straightforward, and I tried to avoid to make
the code less efficient.  For me, it seems to give good results.
Please (Eli), if names and comments are bad English, please feel free to
suggest a better wording.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-Improve-pp-emacs-lisp-code-backquote-form-printi.patch --]
[-- Type: text/x-diff, Size: 4082 bytes --]

From a6ad2e056b7ac136ac766b1fcdb5f59f2f505e15 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Sun, 18 Feb 2024 01:55:54 +0100
Subject: [PATCH] WIP: Improve pp-emacs-lisp-code backquote form printing

* lisp/emacs-lisp/pp.el (pp--quoted-or-unquoted-form-p): New helper
function.
(pp--insert-lisp): Take care of quoted, backquoted and
unquoted expressions; print using an recursive call.
(pp--format-list): Exclude more cases from printing as a function call
by default.  Print lists whose second-last element is an (un)quoting
symbol using dotted list syntax; e.g. (a b . ,c) instead of (a b \, c).
---
 lisp/emacs-lisp/pp.el | 56 ++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/lisp/emacs-lisp/pp.el b/lisp/emacs-lisp/pp.el
index 944dd750839..045cc171eaa 100644
--- a/lisp/emacs-lisp/pp.el
+++ b/lisp/emacs-lisp/pp.el
@@ -430,23 +430,33 @@ pp-emacs-lisp-code
           (replace-match ""))
         (insert-into-buffer obuf)))))

+(defvar pp--quoting-syntaxes
+  `((quote    . "'")
+    (function . "#'")
+    (,backquote-backquote-symbol . "`")
+    (,backquote-unquote-symbol   . ",")
+    (,backquote-splice-symbol    . ",@")))
+
+(defun pp--quoted-or-unquoted-form-p (cons)
+  ;; Return non-nil when CONS has one of the forms 'X, `X, ,X or ,@X
+  (let ((head (car cons)))
+    (and (symbolp head)
+         (assq head pp--quoting-syntaxes)
+         (let ((rest (cdr cons)))
+           (and (consp rest) (null (cdr rest)))))))
+
 (defun pp--insert-lisp (sexp)
   (cl-case (type-of sexp)
     (vector (pp--format-vector sexp))
     (cons (cond
            ((consp (cdr sexp))
-            (if (and (length= sexp 2)
-                     (memq (car sexp) '(quote function)))
-                (cond
-                 ((symbolp (cadr sexp))
-                  (let ((print-quoted t))
-                    (prin1 sexp (current-buffer))))
-                 ((consp (cadr sexp))
-                  (insert (if (eq (car sexp) 'quote)
-                              "'" "#'"))
-                  (pp--format-list (cadr sexp)
-                                   (set-marker (make-marker) (1- (point))))))
-              (pp--format-list sexp)))
+            (let ((head (car sexp)))
+              (if-let (((null (cddr sexp)))
+                       (syntax-entry (assq head pp--quoting-syntaxes)))
+                  (progn
+                    (insert (cdr syntax-entry))
+                    (pp--insert-lisp (cadr sexp)))
+                (pp--format-list sexp))))
            (t
             (prin1 sexp (current-buffer)))))
     ;; Print some of the smaller integers as characters, perhaps?
@@ -470,15 +480,29 @@ pp--format-vector
   (insert "]"))

 (defun pp--format-list (sexp &optional start)
-  (if (and (symbolp (car sexp))
-           (not pp--inhibit-function-formatting)
-           (not (keywordp (car sexp))))
+  (if (not (let ((head (car sexp)))
+             (or pp--inhibit-function-formatting
+                 (not (symbolp head))
+                 (keywordp head)
+                 (let ((l sexp))
+                   (catch 'not-funcall
+                     (while l
+                       (when (or
+                              (not (consp l)) ; SEXP is a dotted list
+                              ;; Is SEXP is of a form like (ELT... . ,X) ?
+                              (pp--quoted-or-unquoted-form-p l))
+                         (throw 'not-funcall t))
+                       (setq l (cdr l)))
+                     nil)))))
       (pp--format-function sexp)
     (insert "(")
     (pp--insert start (pop sexp))
     (while sexp
       (if (consp sexp)
-          (pp--insert " " (pop sexp))
+          (if (not (pp--quoted-or-unquoted-form-p sexp))
+              (pp--insert " " (pop sexp))
+            (pp--insert " . " sexp)
+            (setq sexp nil))
         (pp--insert " . " sexp)
         (setq sexp nil)))
     (insert ")")))
--
2.39.2


[-- Attachment #3: Type: text/plain, Size: 11 bytes --]



Michael.

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>   iarchivedmywholelife@gmail.com,  69168@debbugs.gnu.org
> Date: Thu, 29 Feb 2024 03:30:12 +0100
> 
> Michael Heerdegen <michael_heerdegen@web.de> writes:
> 
> > One from me: I'm working on a second patch for pp that will make
> > printing of backquote forms using the nicer `X syntax.
> 
> Here is the additional patch.  No Wayman, maybe you can try if it gives
> good results?
> 
> This change seems quite straightforward, and I tried to avoid to make
> the code less efficient.  For me, it seems to give good results.
> Please (Eli), if names and comments are bad English, please feel free to
> suggest a better wording.

LGTM, with one minor nit:

> +                              (not (consp l)) ; SEXP is a dotted list
> +                              ;; Is SEXP is of a form like (ELT... . ,X) ?

The "Is SEXP is" part seems to have one "is" too many...

I suggest

  Does SEXP have a form like (ELT... . ,X) ?

Thanks.

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

[Drew Adams]

> > (not (consp l))

Aka (atom l).

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

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

Eli Zaretskii <eliz@gnu.org> writes:

> I suggest
>
>   Does SEXP have a form like (ELT... . ,X) ?

Done, thanks.  Along with Drew's correction.

I have pushed both commits to master and close this report.


Michael.

bug#69168: [BUG] 30.0.50; pp-emacs-lisp-code produces invalid elisp for backquoted forms

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

> Thanks.  Stefan, any comments?

Just that I fully agree with:

>> I think we should print any symbol using `prin1', not `princ' - printing
>> symbols that need quoting without quoting is not useful:

For that same reason, I've been changing several %s into %S in `format`
and `message` arguments.


        Stefan