emacs-devel
[Top][All Lists]
Advanced

[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.

Attachment: signature.asc
Description: PGP signature

Attachment: 0001-Add-cl-map-into.patch
Description: Add cl-map-into


reply via email to

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