[BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[Aaron Madlon-Kay]

Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

Local definitions for `org-entities-user` are not recognized as "safe"
even when they are in the correct format. For instance, including the
following local variables list in an org-mode file will still result in
Emacs prompting you to accept the value:

# Local Variables:
# org-entities-user: (("snowman" "[snowman]" nil "☃" "[snowman]" "[snowman]" "☃"))
# End:

Cause: The defcustom for `org-entities-user` has :safe set to the
function `org-entities--user-safe-p`. This function, however, appears to
be designed to validate a single entry of `org-entities-user`, rather
than the full value. Locally redefining as follows results in the
expected behavior:

(defun org-entities--user-safe-p (v)
  "Non-nil if V is a safe value for `org-entities-user'."
  (seq-every-p
   (lambda (e) (pcase e
            (`nil t)
            (`(,(and (pred stringp)
	                 (pred (string-match-p "\\`[a-zA-Z][a-zA-Z0-9]*\\'")))
               ,(pred stringp) ,(pred booleanp) ,(pred stringp)
               ,(pred stringp) ,(pred stringp) ,(pred stringp))
             t)
            (_ nil)))
   v))

The difference is that the current logic has been supplied as a lambda
to `seq-every-p`.

Thank you,
Aaron

Emacs  : GNU Emacs 30.0.50 (build 1, x86_64-apple-darwin23.1.0, NS appkit-2487.20 Version 14.1.1 (Build 23B81))
of 2023-11-26
Package: Org mode version 9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)

Re: [BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[Aaron Madlon-Kay]

> On Dec 12, 2023, at 20:40, Aaron Madlon-Kay <aaron@madlon-kay.com> wrote:
> 
> Locally redefining as follows results in the expected behavior:

The previous formulation was not very good. Actually it seems that the existing
function is correct except that the list case should be wrapped in (seq …).

(defun org-entities--user-safe-p (v)
  "Non-nil if V is a safe value for `org-entities-user'."
  (pcase v
    (`nil t)
    ((seq `(,(and (pred stringp)
	          (pred (string-match-p "\\`[a-zA-Z][a-zA-Z0-9]*\\'")))
        ,(pred stringp) ,(pred booleanp) ,(pred stringp)
        ,(pred stringp) ,(pred stringp) ,(pred stringp)))
     t)
    (_ nil)))

Regardless I leave the details to the developers.

-Aaron

Re: [BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[Aaron Madlon-Kay]

> On Dec 12, 2023, at 21:01, Aaron Madlon-Kay <aaron@madlon-kay.com> wrote:
> 
>> On Dec 12, 2023, at 20:40, Aaron Madlon-Kay <aaron@madlon-kay.com> wrote:
>> 
>> Locally redefining as follows results in the expected behavior:
> 
> The previous formulation was not very good. Actually it seems that the existing
> function is correct except that the list case should be wrapped in (seq …).

OK this was also wrong, because seq only matches a finite sequence. I couldn’t
find a way to match an arbitrary-length list with pcase, so here’s my final
attempt:

(defun org-entities--user-safe-p (v)
  "Non-nil if V is a safe value for `org-entities-user'."
  (cond
   ((not v) t)
   ((listp v)
    (seq-every-p
     (lambda (e)
       (pcase e
         (`(,(and (pred stringp)
	              (pred (string-match-p "\\`[a-zA-Z][a-zA-Z0-9]*\\'")))
            ,(pred stringp) ,(pred booleanp) ,(pred stringp)
            ,(pred stringp) ,(pred stringp) ,(pred stringp))
          t)
         (_ nil)))
     v))))

This seems to handle all cases correctly, but again of course I leave the
details to the devs (is seq-every-p allowed? is there actually a way to do it
with just pcase?).

Thanks,
Aaron

Re: [BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[Ihor Radchenko]

Aaron Madlon-Kay <aaron@madlon-kay.com> writes:

> OK this was also wrong, because seq only matches a finite sequence. I couldn’t
> find a way to match an arbitrary-length list with pcase, so here’s my final
> attempt:
>
> ...
> This seems to handle all cases correctly, but again of course I leave the
> details to the devs (is seq-every-p allowed? is there actually a way to do it
> with just pcase?).

`seq-every-p' is perfectly fine. I am now aware about a way to do it just
with `pcase'.

May you turn your code into a proper patch?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[Aaron Madlon-Kay]

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

> On Dec 12, 2023, at 23:15, Ihor Radchenko <yantar92@posteo.net> wrote:
> 
> This particular change should fit within TINYCHANGE, I think. So, you do
> not need a copyright assignment (unless you have already exhausted the
> 15LOC limit on Emacs patches, which I do not see in git logs).

Ah, yes, that should be OK then. Please see attached.

Thanks,
Aaron


[-- Attachment #2: 0001-lisp-org-entities.el-Fix-safe-value-predicate-for-or.patch --]
[-- Type: application/octet-stream, Size: 1669 bytes --]

From 0716d90fe9a19fb668fd0fc6890adc565205f8d5 Mon Sep 17 00:00:00 2001
From: Aaron Madlon-Kay <aaron@madlon-kay.com>
Date: Tue, 12 Dec 2023 23:21:22 +0900
Subject: [PATCH] lisp/org-entities.el: Fix safe value predicate for
 org-entities-user

* lisp/org-entities.el (org-entities--user-safe-p): Fix logic to
validate a list of entries, rather than a single entry.

Reported-by: "Aaron Madlon-Kay" <aaron@madlon-kay.com>
Link: https://list.orgmode.org/874jgn7f7s.fsf@localhost/

TINYCHANGE
---
 lisp/org-entities.el | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lisp/org-entities.el b/lisp/org-entities.el
index 91c17f4d6..87d2fbe67 100644
--- a/lisp/org-entities.el
+++ b/lisp/org-entities.el
@@ -41,14 +41,19 @@ defgroup org-entities
 
 (defun org-entities--user-safe-p (v)
   "Non-nil if V is a safe value for `org-entities-user'."
-  (pcase v
-    (`nil t)
-    (`(,(and (pred stringp)
-	     (pred (string-match-p "\\`[a-zA-Z][a-zA-Z0-9]*\\'")))
-       ,(pred stringp) ,(pred booleanp) ,(pred stringp)
-       ,(pred stringp) ,(pred stringp) ,(pred stringp))
-     t)
-    (_ nil)))
+  (cond
+   ((not v) t)
+   ((listp v)
+    (seq-every-p
+     (lambda (e)
+       (pcase e
+         (`(,(and (pred stringp)
+	              (pred (string-match-p "\\`[a-zA-Z][a-zA-Z0-9]*\\'")))
+            ,(pred stringp) ,(pred booleanp) ,(pred stringp)
+            ,(pred stringp) ,(pred stringp) ,(pred stringp))
+          t)
+         (_ nil)))
+     v))))
 
 (defcustom org-entities-user nil
   "User-defined entities used in Org to produce special characters.
-- 
2.43.0

Re: [BUG] "Safe" local values for org-entities-user not recognized as such [9.6.11 (release_9.6.11 @ /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/org/)]

[Ihor Radchenko]

Aaron Madlon-Kay <aaron@madlon-kay.com> writes:

>> This particular change should fit within TINYCHANGE, I think. So, you do
>> not need a copyright assignment (unless you have already exhausted the
>> 15LOC limit on Emacs patches, which I do not see in git logs).
>
> Ah, yes, that should be OK then. Please see attached.

Thanks!
Applied, onto bugfix.
Fixed.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1ec18b8eb

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>