Skip to content

Enhance the add arity refactoring #541

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

Merged
merged 1 commit into from
Jul 25, 2019
Merged

Conversation

anthonygalea
Copy link
Contributor

@anthonygalea anthonygalea commented Jul 23, 2019

Just came across a case which the current clojure-add-arity refactoring does not handle well i.e. when a defn lies within a reader conditional. This pull request modifies clojure-add-arity to support this case.

  • The commits are consistent with our [contribution guidelines][1].
  • You've added tests (if possible) to cover your change(s). Bugfix, indentation, and font-lock tests are extremely important!
  • You've run M-x checkdoc and fixed any warnings in the code you've written.
  • You've updated the changelog (if adding/changing user-visible functionality).
  • You've updated the readme (if adding/changing user-visible functionality).

@bbatsov
Copy link
Member

bbatsov commented Jul 24, 2019

We'll have to mention the bugfix in the changelog as well. Btw, does the command handle properly letfn and things like proxy, reify, defprotocol?

@anthonygalea anthonygalea force-pushed the master branch 2 times, most recently from 8f0c99e to c6b5122 Compare July 24, 2019 16:05
@anthonygalea
Copy link
Contributor Author

Can you think of any other cases @bbatsov?

@anthonygalea anthonygalea changed the title Add reader conditional support to the add arity refactoring. Enhance the add arity refactoring Jul 24, 2019
@yuhan0
Copy link
Contributor

yuhan0 commented Jul 24, 2019

What about defmacro? And to some extentdefmethod, although adding an arity to a single method probably isn't very useful.
Also the anonymous fnform can take multiple arities and would be nice if it was supported too :)

@anthonygalea
Copy link
Contributor Author

Many thanks for your feedback @yuhan0. Wasn't even aware that it was possible to have multi-arity in a defmethod.

Support reader conditionals, letfn, fn, defmacro, defmethod, defprotocol, reify
and proxy in addition to defn.
@bbatsov bbatsov merged commit f23eb20 into clojure-emacs:master Jul 25, 2019
@bbatsov
Copy link
Member

bbatsov commented Jul 25, 2019

Nicely done! Now I've started wonder if we need a remove arity counterpart. :-)

@anthonygalea
Copy link
Contributor Author

Hadn't thought of that. Guessing it will add very little value, but we'll never know until it's implemented. I'll give it a shot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants