[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add cl-map-into, revision 3
From: |
akater |
Subject: |
Re: [PATCH] Add cl-map-into, revision 3 |
Date: |
Sat, 09 Oct 2021 02:46:17 +0000 |
Eli Zaretskii <eliz@gnu.org> writes:
>> +(cl-defmacro cl--do-seq-type-signature ((type-var signature &optional
>> result)
>> + &body body)
>> + "With TYPE-VAR bound to sequence type, evaluate BODY forms. Return
>> RESULT.
>
> The first line of a doc string should be a single sentence, not longer
> than 79 characters.
>
>> +(defun cl--make-map-into-mapper (signature &optional do-not-compile)
>> + "Return mapper for `cl-map-into' specialized on arglists of type
>> +encoded by SIGNATURE.
>
> Same here.
This one is not public interface though. But I fixed this one
nevertheless.
>> +(defun cl-map-into (target function &rest sequences)
>> + "Common Lisp's map-into.
>
> The first line of a doc string of a public interface should describe
> the arguments, at least the mandatory ones.
Done.
Changes:
- NEWS (29) entry
- entry in cl.texi
- supported are list, vector, bool-vector, string
- some more tests
- make-mapper code is simplified
- “target” renamed to “result-sequence” because that's the way it is in
cl spec
- clean docstrings
Three points remain.
- Regarding “do-not-compile” argument in make-mapper. It would be
better to have “compile” argument instead, with 3 possible values:
nil, byte-compile, native-compile. Native-compile seems to work right
now but I'm just getting acquainted with the feature, it's not going
smooth, and I'm not sure whether native-compile can be used by default
in cl-map-into. If cl-map-into won't make it into Emacs 28, I suggest
using native-compile right away, for ease of experimentation since
nothing depends on cl-map-into right now.
- I prefer providing examples for functions, including “internal” ones;
most of the time examples come naturally during development so it's
better to use them than to let them go to waste. Usually examples are
nowhere to submit; I thus often leave them in docstrings, especially
when it comes to “internal” functions. This is the case with this
patch. While people do this sometimes, and there's even a Common Lisp
library that addresses this technique in some way, I'm not sure if
this is appropriate style.
- I left a comment block in the beginning. Since the existing mapper in
cl-extra is buggy, I'd rather have at least some of the comments
remain. It will get into sight of more people this way than a mere
bug in the tracker, and implmenting new similar dispatchers seems
straightforward. I'll report the bug in coming days unless the bug is
already there. Also commented are (possible) type declarations. I
think they convey something useful as well.
signature.asc
Description: PGP signature
0001-Add-cl-map-into.patch
Description: Add cl-map-into