bug#40600: 27.0.90; M-x strokes-list-strokes error

bug#40600: 27.0.90; M-x strokes-list-strokes error

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

I browsed Emacs source repository and found strokes.el package.
This is very awesome but I found a bug.

When two or more strokes are defined including string returning strokes, 
M-x strokes-list-strokes causes an error.

Reproduce steps:
1. emacs -Q
2. M-x strokes-global-set-stroke (and define a stroke and corresponding command(emacs-version))
3. M-x strokes-global-set-stroke-string (and define a stroke and corresponding string(emacs))
4. M-x strokes-list-strokes => error message
;; when (setq debug-on-error t)
Debugger entered--Lisp error: (wrong-type-argument symbolp "emacs")
  symbol-name("emacs")
  strokes-alphabetic-lessp((((6 . 0) (5 . 0) (4 . 0) (3 . 0) (2 . 0) (1 . 0) (0 . 1) (0 . 2) (0 . 3) (0 . 4) (0 . 5) (1 . 5) (2 . 6) (3 . 6) (4 . 6) (4 . 5) (5 . 4) (5 . 3) (6 . 3) (6 . 2) (6 . 1) (6 . 2) (6 . 3) (6 . 4) (7 . 5) (7 . 6) (8 . 6) (8 . 7) (8 . 8)) . emacs-version) (((5 . 2) (5 . 1) (4 . 1) (4 . 0) (3 . 0) (2 . 0) (2 . 1) (1 . 1) (1 . 2) (0 . 2) (0 . 3) (0 . 4) (0 . 5) (0 . 6) (0 . 7) (1 . 7) (2 . 7) (3 . 7) (4 . 7) (5 . 7) (6 . 7) (6 . 6) (7 . 5) (7 . 4) (7 . 5) (7 . 6) (7 . 7) (8 . 7) (8 . 8)) . "emacs"))
  sort(((((5 . 2) (5 . 1) (4 . 1) (4 . 0) (3 . 0) (2 . 0) (2 . 1) (1 . 1) (1 . 2) (0 . 2) (0 . 3) (0 . 4) (0 . 5) (0 . 6) (0 . 7) (1 . 7) (2 . 7) (3 . 7) (4 . 7) (5 . 7) (6 . 7) (6 . 6) (7 . 5) (7 . 4) (7 . 5) (7 . 6) (7 . 7) (8 . 7) (8 . 8)) . "emacs")) strokes-alphabetic-lessp)
  strokes-list-strokes(nil)
  funcall-interactively(strokes-list-strokes nil)
  call-interactively(strokes-list-strokes record nil)
  command-execute(strokes-list-strokes record)
  execute-extended-command(nil "strokes-list-strokes" "strokes-lis")
  funcall-interactively(execute-extended-command nil "strokes-list-strokes" "strokes-lis")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

Thanks.

--
tsuucat

bug#40600: [PATCH] 27.0.90; M-x strokes-list-strokes error

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

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

I made a patch.
Please see an attachment.

--
tsuucat

[-- Attachment #2: 0001-Fix-comparing-command-names-in-strokes.el-bug-40600.patch --]
[-- Type: application/octet-stream, Size: 1292 bytes --]

From a95cf228b1cebbea0a0a56f2804cc0c42035c8a7 Mon Sep 17 00:00:00 2001
From: Masahiro Nakamura <tsuucat@icloud.com>
Date: Tue, 14 Apr 2020 22:37:17 +0900
Subject: [PATCH] Fix comparing command names in strokes.el (bug#40600)

* lisp/strokes.el (strokes-alphabetic-lessp): Check arguments with symbolp
because arguments may be strings.
---
 lisp/strokes.el | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lisp/strokes.el b/lisp/strokes.el
index 7a887445..5f40ffcc 100644
--- a/lisp/strokes.el
+++ b/lisp/strokes.el
@@ -1373,8 +1373,12 @@ strokes-list-strokes
 
 (defun strokes-alphabetic-lessp (stroke1 stroke2)
   "Return t if STROKE1's command name precedes STROKE2's in lexicographic order."
-  (let ((command-name-1 (symbol-name (cdr stroke1)))
-	(command-name-2 (symbol-name (cdr stroke2))))
+  (let ((command-name-1 (if (symbolp (cdr stroke1))
+                            (symbol-name (cdr stroke1))
+                          (prin1-to-string (cdr stroke1))))
+        (command-name-2 (if (symbolp (cdr stroke2))
+                            (symbol-name (cdr stroke2))
+                          (prin1-to-string (cdr stroke2)))))
     (string-lessp command-name-1 command-name-2)))
 
 (defvar strokes-mode-map
-- 
2.21.0

bug#40600: [PATCH] 27.0.90; M-x strokes-list-strokes error

[Noam Postavsky]

tsuucat <tsuucat@icloud.com> writes:

> -  (let ((command-name-1 (symbol-name (cdr stroke1)))
> -	(command-name-2 (symbol-name (cdr stroke2))))
> +  (let ((command-name-1 (if (symbolp (cdr stroke1))
> +                            (symbol-name (cdr stroke1))
> +                          (prin1-to-string (cdr stroke1))))

Why use prin1-to-string?

> +        (command-name-2 (if (symbolp (cdr stroke2))
> +                            (symbol-name (cdr stroke2))
> +                          (prin1-to-string (cdr stroke2)))))
>      (string-lessp command-name-1 command-name-2)))

Actually, since string-lessp accepts symbols, an easier fix would be to
just drop the calls to symbol-name.

bug#40600: [PATCH] 27.0.90; M-x strokes-list-strokes error

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

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

> Why use prin1-to-string?

That’s simply because I imitate the code in strokes-list-strokes function.

> Actually, since string-lessp accepts symbols, an easier fix would be to
> just drop the calls to symbol-name.


Thanks for the advice! I made another patch and checked this  works well.

--
tsuucat

[-- Attachment #2: 0001-Fix-comparing-command-names-in-strokes.el-bug-40600.patch --]
[-- Type: application/octet-stream, Size: 1032 bytes --]

From af31735e07528973392cb1e163f9f9e1682951e4 Mon Sep 17 00:00:00 2001
From: Masahiro Nakamura <tsuucat@icloud.com>
Date: Tue, 14 Apr 2020 22:37:17 +0900
Subject: [PATCH] Fix comparing command names in strokes.el (bug#40600)

* lisp/strokes.el (strokes-alphabetic-lessp): Simply call string-lessp
because the cdr of the argument may be string.
---
 lisp/strokes.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/strokes.el b/lisp/strokes.el
index 7a887445..e511a63f 100644
--- a/lisp/strokes.el
+++ b/lisp/strokes.el
@@ -1373,9 +1373,7 @@ strokes-list-strokes
 
 (defun strokes-alphabetic-lessp (stroke1 stroke2)
   "Return t if STROKE1's command name precedes STROKE2's in lexicographic order."
-  (let ((command-name-1 (symbol-name (cdr stroke1)))
-	(command-name-2 (symbol-name (cdr stroke2))))
-    (string-lessp command-name-1 command-name-2)))
+  (string-lessp (cdr stroke1) (cdr stroke2)))
 
 (defvar strokes-mode-map
   (let ((map (make-sparse-keymap)))
-- 
2.21.0

bug#40600: [PATCH] 27.0.90; M-x strokes-list-strokes error

[Noam Postavsky]

tags 40600 fixed
close 40600 28.1
quit

tsuucat <tsuucat@icloud.com> writes:

> Thanks for the advice! I made another patch and checked this  works well.

Thanks, pushed to master.

[1: be77a68d52]: 2020-04-16 20:47:35 -0400
  Fix comparing command names in strokes.el (bug#40600)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=be77a68d527223f7f276e94e16fe05b49846c7a3