Re: [PATCH] pcase.el: Add cl-type and type patterns

[Adam Porter <adam@alphapapa.net>]

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

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

> A couple of comments about the documentation parts below.  I'll leave
> the main part to other reviewers.

>> +@item (cl-type @var{type})
>> +Matches if @var{expval} is of type @var{type}, which is a symbol or
>> +list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
>
> Please don't use @ref with this "HTML-style", it produces bad results
> in Info.  Instead, use a slightly more wordy
>
>   ... as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
>   Extensions}).

Sure, I've changed it.

>> +Example: @code{(cl-type integer)}
>> +Example: @code{(cl-type (integer 0 10))}
>
> Examples should be in a @lisp..@end lisp block, and then you can drop
> the @code markup, which is applied automatically.

Ok, I've changed that two-line example to use "@lisp...@end lisp".  I've
left the one-line example in the second patch in the "Example:
@code{...}" format, which matches all of the other single-line examples
in control.texi.  If you want me to change that one as well, let me
know.

>> +*** Added Pcase 'cl-type' pattern.
>              ^^^^^
> "pcase", without capitalization.

The "Pcase" there was actually redundant, being present in the parent
heading, so I've removed it.

Please see the attached, updated patches.  Thanks for your help.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add cl-typep pattern --]
[-- Type: text/x-diff, Size: 3310 bytes --]

From 2ecda2e27246e3bff2d0cc7c2f89a77c765e6aa5 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH 1/2] * lisp/emacs-lisp/cl-macs.el: Add cl-type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'cl-type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-cl-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

With thanks to Stefan Monnier and Eli Zaretskii for their guidance.
---
 doc/lispref/control.texi            | 10 ++++++++++
 etc/NEWS                            |  5 +++++
 lisp/emacs-lisp/cl-macs.el          |  8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 33 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 22b665b..fc91d6c 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,16 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (cl-type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a symbol or
+list as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
+Extensions}).  Examples:
+
+@lisp
+(cl-type integer)
+(cl-type (integer 0 10))
+@end lisp
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.  The test can be negated with the syntax
diff --git a/etc/NEWS b/etc/NEWS
index 923cfcc..3af842c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,6 +515,11 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
+*** Added 'cl-type' pattern.
+The new 'cl-type' pattern compares types using 'cl-typep', which allows
+comparing simple types like '(cl-type integer)', as well as forms like
+'(cl-type (integer 0 10))'.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index cff4368..4cfc07a 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3623,6 +3623,14 @@ cl-struct-slot-value
                         "use `with-eval-after-load' instead." "28.1")
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+;;;###autoload
+(pcase-defmacro cl-type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a symbol or list as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 2120139..02d3878 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,4 +100,14 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-cl-type ()
+  (should (equal (pcase 1
+                   ((cl-type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((cl-type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((cl-type notatype) 'integer))))
+
 ;;; pcase-tests.el ends here.
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Add type pattern --]
[-- Type: text/x-diff, Size: 3766 bytes --]

From bf8ad829e168fde14f649f1ce167a2a4c9c5e115 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Fri, 16 Jul 2021 04:29:21 -0500
Subject: [PATCH 2/2] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el ((pcase-defmacro type)): Add 'type'
pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

* etc/NEWS: Update news.

With thanks to Stefan Monnier and Eli Zaretskii for their guidance.
---
 doc/lispref/control.texi            |  8 ++++++++
 etc/NEWS                            | 12 ++++++++----
 lisp/emacs-lisp/pcase.el            | 11 +++++++++++
 test/lisp/emacs-lisp/pcase-tests.el |  7 +++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index fc91d6c..137ce33 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,14 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}, i.e. a type for which a
+corresponding @code{typep} or @code{type-p} predicate is defined.  For
+matching structs defined with @code{cl-defstruct}, the @code{cl-type}
+matcher should be used instead.
+
+Example: @code{(type integer)}
+
 @item (cl-type @var{type})
 Matches if @var{expval} is of type @var{type}, which is a symbol or
 list as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
diff --git a/etc/NEWS b/etc/NEWS
index 3af842c..62659e2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,10 +515,14 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
-*** Added 'cl-type' pattern.
-The new 'cl-type' pattern compares types using 'cl-typep', which allows
-comparing simple types like '(cl-type integer)', as well as forms like
-'(cl-type (integer 0 10))'.
+*** Added 'type' and 'cl-type' patterns.
+
+The new 'type' pattern compares simple types using the corresponding
+'typep' or 'type-p' predicate, e.g. '(type integer)'.
+
+The new 'cl-type' pattern compares types using 'cl-typep', which
+allows comparing simple types like '(cl-type integer)', as well as
+forms like '(cl-type (integer 0 10))'.
 
 +++
 ** profiler.el
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 006517d..c7e2027 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -1041,5 +1041,16 @@ let
 ;;   (declare (debug (form)))
 ;;   `(pred (lambda (_) ,expr)))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE.
+This checks using a predicate named `TYPEp' or `TYPE-p', as
+appropriate.  For matching structs defined with `cl-defstruct',
+the `cl-type' pattern matcher should be used instead."
+  (let* ((type (symbol-name type))
+         (pred (or (intern-soft (concat type "p"))
+                   (intern-soft (concat type "-p"))
+                   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 02d3878..9184485 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,6 +100,13 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 (ert-deftest pcase-tests-cl-type ()
   (should (equal (pcase 1
                    ((cl-type integer) 'integer))
-- 
2.7.4