Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/avoid recursion #362

Merged
merged 2 commits into from
Jun 2, 2021

Conversation

perweij
Copy link
Contributor

@perweij perweij commented Jan 5, 2021

The recursive nature of -partition-after-pred causes problems when processing long lists where these limits are exceeded:
max-lisp-eval-depth
max-specpdl-size

I have tried to rewrite it as an iterative macro instead. It seems to work, and passes the tests, but if you have time, please check the code for possible improvements.

Copy link
Collaborator

@basil-conto basil-conto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it's always good to avoid recursion in Elisp if possible! :) But this still needs a bit of work. ;)

This probably counts as a non-trivial change (in terms of lines of code), so in order to install it we'd need you to assign the copyright to the Free Software Foundation, as Dash is copyrighted by the FSF. Do you already have such an assignment on file? If not, would you be willing to submit one? The process is relatively simple and painless, and once it's done you'll be able to contribute to Emacs itself and other core packages as well. See (info "(emacs) Copyright Assignment") for more info on this. You can find the relevant form here: https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future.

TIA.

dash.el Outdated Show resolved Hide resolved
dash.el Outdated Show resolved Hide resolved
dash.el Outdated Show resolved Hide resolved
@basil-conto basil-conto added copyright Waiting for FSF copyright assignment enhancement Suggestion to improve or extend existing behavior labels Jan 5, 2021
@perweij
Copy link
Contributor Author

perweij commented Jan 5, 2021

Hi! Many thanks for checking this so quickly!

If not, would you be willing to submit one?

Done - I'll update here when I have received a reply.

@perweij
Copy link
Contributor Author

perweij commented Jan 6, 2021

sorry @basil-conto I did not just deliberately request a new review - misunderstood the refresh icon, thought it would reload stuff but instead it sent you a request. Again, sorry.

@basil-conto
Copy link
Collaborator

No worries! Now I know what it does too :)

@basil-conto
Copy link
Collaborator

Done - I'll update here when I have received a reply.

What's the status on this @perweij? If you're waiting more than a week or two for a response from the copyright clerk, feel free to ping them.

@basil-conto
Copy link
Collaborator

Ping.

@perweij
Copy link
Contributor Author

perweij commented May 25, 2021

Hi! Now I have assigned the copyright to the Free Software Foundation. The process is completed, and I have received the final PDF file from FSF, so we should be able to proceed. Sorry for the looong delay!

@basil-conto basil-conto removed the copyright Waiting for FSF copyright assignment label May 25, 2021
@basil-conto basil-conto added this to the 2.19.0 milestone Jun 2, 2021
@basil-conto basil-conto added this to In Progress in Release 2.19.0 Jun 2, 2021
@basil-conto basil-conto self-assigned this Jun 2, 2021
perweij and others added 2 commits June 2, 2021 17:01
* dash.el (--partition-after-pred): New anaphoric macro.
(-partition-after-pred): Use it.
* dev/examples.el (-partition-after-pred): Test it.
* dash.el (--partition-after-pred): Simplify.  Extend docstring.
(-partition-after-pred): Extend docstring.
* dev/examples.el (-partition-after-pred): Extend tests.

* README.md:
* dash.texi: Regenerate docs.

Re: magnars#362.
@basil-conto basil-conto merged commit aab346e into magnars:master Jun 2, 2021
@basil-conto
Copy link
Collaborator

Sorry for the looong delay!

No worries, and thanks for the nice and efficient implementation!

@basil-conto basil-conto moved this from In Progress to Done in Release 2.19.0 Jun 3, 2021
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants