[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Emacs-diffs] master 387e1e1: New version of `seq-let' based on a pc
|
From: |
Stefan Monnier |
|
Subject: |
Re: [Emacs-diffs] master 387e1e1: New version of `seq-let' based on a pcase pattern |
|
Date: |
Mon, 11 May 2015 00:15:05 -0400 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) |
> + (pcase-defmacro seq (bindings)
> + `(and ,@(seq--make-pcase-bindings bindings)))
If you put a docstring in there, it will appear in "C-h f pcase RET", so
you can use it to document the particular format you support.
Also I see the following problems:
- You only accept the form (seq <binding>) rather than (seq <binding1>
<binding2> ...), so typical cases will have to use (seq (a b)) instead
(seq a b).
- The above pcase pattern doesn't check that it's indeed a `seq'.
You should add a (pred seq-p). It will automatically be optimized
away in `pcase-let', but is indispensable for the `pcase' situation to
do the right thing.
> +(defun seq--make-pcase-bindings (args &optional bindings nested-indexes)
[...]
> + (seq-doseq (name args)
> + (unless rest-marker
> + (pcase name
> + ((pred seq-p)
IIUC, this means that (pcase-let (((seq a (seq b c)) <obj>)) <body>)
will bind the `seq' variable to the first element of the nested sequence
(and `a' to the second and `b' to the third), whereas I think it should
bind `b' to the first element of the nested sequence (which is what
would happen if you simply removed this branch of this `pcase').
I think removing this case will also remove the need for seq--nested-elt.
> + (push `(app (seq--reverse-args #'seq--nested-elt
> + (reverse (cons ,index
> ',nested-indexes)))
> + ,name)
This reverse plus seq--reverse-args business seems
hideously inefficient. Why do you need that?
Stefan
- Re: [Emacs-diffs] master 387e1e1: New version of `seq-let' based on a pcase pattern,
Stefan Monnier <=