[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] pcase.el: Add cl-type and type patterns
From: |
Stefan Monnier |
Subject: |
Re: [PATCH] pcase.el: Add cl-type and type patterns |
Date: |
Fri, 16 Jul 2021 18:41:47 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> +@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}.
Rather than mention "symbol or list", I'd only refer to `cl-typep`,
maybe something like:
Matches if @var{expval} is of type @var{type}, which is type descriptor
as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
The rest of the first patch looks good to me.
> +(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)))
I'm not convinced this second patch is worth the trouble:
(type FOO)
is not significantly shorter or simpler than
(pred FOO-p)
but it is inherently brittle because `intern-soft` may
return non-nil even though there's no predicate by that name, and it may
also let you use a "type" which really isn't one such as
`cl--struct-name` or `looking-at`.
`cl-type` provides that functionality in a cleaner and more
reliable way (it still also relies to some extent on similar heuristic
as your code, admittedly, but I've been working to get rid of it).
Stefan