Skip to content

Replaced all occurrences of even? with evenp#342

Open
cfraizer wants to merge 1 commit intomagnars:masterfrom
cfraizer:master
Open

Replaced all occurrences of even? with evenp#342
cfraizer wants to merge 1 commit intomagnars:masterfrom
cfraizer:master

Conversation

@cfraizer
Copy link
Copy Markdown

Also replaced occurrences of odd? with oddp.

Emacs Lisp provides evenp and oddp and apparently the use of (not defined) functions even? and odd? caused confusion for some readers. [I won't cry if you ignore this pull request.]

Also replaced occurrences of `odd?` with `oddp`.
@basil-conto
Copy link
Copy Markdown
Collaborator

Emacs Lisp provides evenp and oddp

Only if you load the cl.el library, which is deprecated, at runtime.

the use of (not defined) functions even? and odd?

They're defined in the file that generates the examples, dev/examples.el.

That said, it's always irked me that the output of this file does not include their definitions.

Instead of using deprecated cl.el functions, my suggestion would therefore be to either change the examples so that they use some other built-in functions; use the corresponding cl-lib.el definitions; or change the example generation so that the definitions used therein are also published. My personal favourite would be the first option.

@basil-conto
Copy link
Copy Markdown
Collaborator

Note also that you shouldn't modify README.md, as it is generated from other files, e.g. readme-template.md and dev/examples-to-docs.el.

@magnars
Copy link
Copy Markdown
Owner

magnars commented Aug 11, 2020

How about 4) Rename to -even? and -odd? and include them in dash proper? Not because they are a perfect match for the dash API, but because they are slightly useful and would make running the examples easier?

@basil-conto
Copy link
Copy Markdown
Collaborator

Is it really worth further blurring the boundaries of the Dash API for the sake of how and which examples are published?

I'm obviously not going to oppose the decision, I just think there are simpler solutions.

@magnars
Copy link
Copy Markdown
Owner

magnars commented Aug 11, 2020 via email

@basil-conto
Copy link
Copy Markdown
Collaborator

It wasn’t a decision. It was a suggestion. As implied by the question marks.

Sorry, I didn't mean to come across as accusatory. I only meant to say that I'm ultimately fine with any of the solutions, but don't think the generation of examples is worth going into too much trouble for.

@cfraizer
Copy link
Copy Markdown
Author

cfraizer commented Aug 16, 2020

Thanks for all the feedback.

I'll revert the changes to the README.md (of course)

And, if you concur, I'll go with @basil-conto 's first suggestion and alter the examples to use, say integerp or zerop? [I'm open to other suggestions and I'm okay if you suggest something else after seeing them changed.]

basil-conto added a commit that referenced this pull request Jan 17, 2021
* dev/examples.el (odd?): Remove function.
(-drop-while, -partition-after-pred, -partition-before-pred)
(-each-while, -each-r-while): Replace odd? with built-in
functions (#342).

* README.md:
* dash.texi: Regenerate docs.
@basil-conto
Copy link
Copy Markdown
Collaborator

And, if you concur, I'll go with @basil-conto 's first suggestion and alter the examples to use, say integerp or zerop? [I'm open to other suggestions and I'm okay if you suggest something else after seeing them changed.]

I've since been chipping away at the non-standard functions whenever I was changing tests for other reasons, and finally odd? started hanging low enough that I just eliminated the handful of cases left in commit ce4a344.

I'm not going to tackle even? or square very soon, so would you like to rebase this branch off master and continue from there?

I think such a cumulative change should be exempt from copyright paperwork, but if you'd like to continue contributing to Dash, Emacs, or other FSF-copyrighted projects, then I'd encourage you to start the copyright assignment process, if you haven't already :). If you're unfamiliar with this, see (info "(emacs) Copyright Assignment"). Thanks.

@basil-conto basil-conto added the enhancement Suggestion to improve or extend existing behavior label Jan 17, 2021
@basil-conto basil-conto self-assigned this Jan 17, 2021
@basil-conto basil-conto added this to the 2.19.0 milestone Feb 15, 2021
@basil-conto basil-conto removed their assignment Feb 15, 2021
@basil-conto basil-conto modified the milestones: 2.19.0, 2.20.0 Jun 29, 2021
@basil-conto
Copy link
Copy Markdown
Collaborator

Emacs Lisp provides evenp and oddp

Only if you load the cl.el library, which is deprecated, at runtime.

Update: Emacs 31 now provides evenp and oddp OOTB.

My personal preference remains to limit examples in Dash docs to using the dash.el API and vanilla Elisp that works in all supported Emacs versions.

Hopefully someone would still like to make this happen (@cfraizer ?) :). Otherwise I'll get to it at some point™...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Suggestion to improve or extend existing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants