Skip to content

Add unwind refactoring commands #88

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
Apr 29, 2025

Conversation

rrudakov
Copy link
Contributor

@rrudakov rrudakov commented Apr 28, 2025

The new commands work the same as in clojure-mode with one improvement (hopefully):

In clojure-mode unwinding form (-> foo bar) would produce (-> (bar foo)), so you need to run unwind command one more time to get rid of threading macro. In clojure-ts-mode it will produce (bar foo) immediately. Because of that I had to adapt tests from clojure-mode a little bit.


Before submitting a PR mark the checkboxes for the items you've done (if you
think a checkbox does not apply, then leave it unchecked):

  • The commits are consistent with our contribution guidelines.
  • 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).

Thanks!

@rrudakov rrudakov force-pushed the feature/threading-refactoring branch 2 times, most recently from 0759b39 to 2db43a4 Compare April 28, 2025 10:11
@bbatsov
Copy link
Member

bbatsov commented Apr 28, 2025

Apart from my small inline remarks I can suggest also:

  • to add a command that displays the unwound version in a separate buffer (I think this will be useful for debug purposes)
  • the inverse command (that convert the normal presentation in a threaded one - I'm guessing that's more common refactoring in practice)

Overall, the PR looks good to me, though.

@rrudakov
Copy link
Contributor Author

Apart from my small inline remarks I can suggest also:

  • to add a command that displays the unwound version in a separate buffer (I think this will be useful for debug purposes)

Could you please suggest a command name for that?

  • the inverse command (that convert the normal presentation in a threaded one - I'm guessing that's more common refactoring in practice)

Sure. I'm planning to port refactoring commands from clojure-mode and I'm going from top to bottom one thing at the time (to avoid big PRs). It happens that unwind was first :)

Overall, the PR looks good to me, though.

Thanks! I'll fix all the issues.

@rrudakov rrudakov force-pushed the feature/threading-refactoring branch from 2db43a4 to f6ef592 Compare April 28, 2025 12:04
@rrudakov rrudakov requested a review from bbatsov April 28, 2025 12:26
@bbatsov
Copy link
Member

bbatsov commented Apr 28, 2025

Could you please suggest a command name for that?

Perhaps something like clojure-ts-show-unwound? Or clojure-ts-unwind-to-buffer? Obviously people can get the same result using cider-macroexpand, but I'm guessing the people who use the refactoring commands in clojure-mode, probably are not heavy CIDER users.

@bbatsov
Copy link
Member

bbatsov commented Apr 28, 2025

Sure. I'm planning to port refactoring commands from clojure-mode and I'm going from top to bottom one thing at the time (to avoid big PRs). It happens that unwind was first :)

I figured something like this was the case. :-)

@rrudakov
Copy link
Contributor Author

Could you please suggest a command name for that?

Perhaps something like clojure-ts-show-unwound? Or clojure-ts-unwind-to-buffer? Obviously people can get the same result using cider-macroexpand, but I'm guessing the people who use the refactoring commands in clojure-mode, probably are not heavy CIDER users.

After giving it some thoughts, I think it would be better to implement in a separate PR. Ideally this would require defining a new minor mode clojure-ts-popup-buffer-mode, write some functions to setup buffer name, major-mode and bind q to quit-buffer, perform unwind command in a temp buffer and insert the result. @bbatsov maybe you could suggest a better idea?

@rrudakov rrudakov force-pushed the feature/threading-refactoring branch from f6ef592 to 97bde83 Compare April 29, 2025 09:20
@bbatsov
Copy link
Member

bbatsov commented Apr 29, 2025

After giving it some thoughts, I think it would be better to implement in a separate PR. Ideally this would require defining a new minor mode clojure-ts-popup-buffer-mode, write some functions to setup buffer name, major-mode and bind q to quit-buffer, perform unwind command in a temp buffer and insert the result. @bbatsov maybe you could suggest a better idea?

No, that's more or less what I normally do in such situations. It's fairly simple and standard - you can use cider-macroexpansion.el as a point of reference. Let's have this as a separate PR.

@bbatsov bbatsov merged commit 4bdd7f2 into clojure-emacs:main Apr 29, 2025
3 checks passed
@rrudakov rrudakov deleted the feature/threading-refactoring branch April 29, 2025 12:10
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.

2 participants