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

Add defcustom for major-mode remapping #64

Merged
merged 3 commits into from
Mar 8, 2025

Conversation

yuhan0
Copy link
Contributor

@yuhan0 yuhan0 commented Mar 6, 2025

Closes #63

Naming of defcustom / commands open to change, if alright I can go ahead and edit the changelog/Readme


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):

  • 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!

@bbatsov
Copy link
Member

bbatsov commented Mar 7, 2025

Looks good.

I noticed clojure-ts-mode also adds separate modes and mappings for jank and ClojureDart, and I think clojure-mode that just maps them to clojure-mode, but we can address this discrepancy.

@@ -119,6 +119,18 @@ double quotes on the third column."
:safe #'booleanp
:package-version '(clojure-ts-mode . "0.2.3"))

(defcustom clojure-ts-auto-redirect t
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps clojure-ts-auto-remap or clojure-ts-auto-remap-modes would be slightly better name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've renamed it to clojure-ts-auto-remap, wasn't sure if it should have a -p suffix being a boolean? Feel free to change it further (maybe the commands too, clojure-ts-activate -> clojure-ts-activate-remapping)

@yuhan0
Copy link
Contributor Author

yuhan0 commented Mar 8, 2025

I've removed the commit which uses defcustom setter to trigger activate/deactivate calls on setopt, after a few attempts at fixing it was still causing recursive load errors when being called on initial package load.

@bbatsov bbatsov merged commit f5f377a into clojure-emacs:main Mar 8, 2025
0 of 3 checks passed
@bbatsov
Copy link
Member

bbatsov commented Mar 8, 2025

Okay, I guess that will do for now. I thought a bit too late that probably those commands shouldn't do anything if clojure-mode is not around, but I doubt anyone would call them if they don't use clojure-mode to some extent.

@rrudakov
Copy link
Contributor

@bbatsov
Copy link
Member

bbatsov commented Mar 10, 2025

Good to know, I'll update the code to use it.

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