emacs-devel
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]