Add -separate-multi and --separate-multi.#299
Add -separate-multi and --separate-multi.#299Luis-Henriquez-Perez wants to merge 3 commits intomagnars:masterfrom Luis-Henriquez-Perez:master
-separate-multi and --separate-multi.#299Conversation
basil-conto
left a comment
basil-conto
left a comment
There was a problem hiding this comment.
Just some docstring nitpicks from me.
| (defun -separate-multi (preds list) | ||
| "Return a list of ((-filter PRED1 LIST) (-filter PRED2 LIST) ... REST), in one | ||
| pass through the list. PREDS is a list of functions and REST are elements of | ||
| LIST that are unmatched by any PREDS." |
There was a problem hiding this comment.
Please follow the Emacs docstring conventions as documented under (elisp) Documentation Tips. In particular: the first line should be a brief but complete sentence; the subsequent body of the docstring should not be indented; and each full stop at the end of a sentence should be followed by two spaces.
There was a problem hiding this comment.
Thanks for this, I just added more commits to fix this. Do you think I should take out the ((-filter PRED1 LIST)...) part? Is it confusing because REST is not a parameter? If I leave that in I can't fit in one pass through the list on the first line. As it is I separated it into it's own line.
| (--map (nreverse (cdr it)) preds)) | ||
|
|
||
| (defmacro --separate-multi (forms list) | ||
| "Anaphoric form of -separate-multi." |
There was a problem hiding this comment.
Please quote symbol references as `-separate-multi' so that the help system can hyperlink them, as per the aforementioned conventions.
|
As I mentioned on #230 (comment), please consider making the return value a cons of two lists: lists of matching items in the CAR, and non-matching items in the CDR. This would make accessing the results and determining whether there are matching and non-matching items simpler and faster. |
|
Why is there a merge commit? |
Fuco1
left a comment
Fuco1
left a comment
There was a problem hiding this comment.
Just a review to block merging until all the comments are solved
|
@basil-conto can you start a review with a change request? I'm not sure if people outside of the repo can do that. We could probably add you to collaborators anyway. |
That was me fixing the comment problems @basil-conto mentioned. This is my first pull request so let me know if I messed something up trying to fix things. |
4 participants
Concerning issue #230.