Skip to content

Pass fully qualified symbol to clojure-ts-get-indent-function #72

New issue

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

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

Already on GitHub? # to your account

Merged

Conversation

rrudakov
Copy link
Contributor

@rrudakov rrudakov commented Apr 5, 2025

When I started testing it with CIDER I realized that we have to pass fully qualified symbol name to cider--get-symbol-indent, otherwise it won't resolve it and won't return indentation metadata.


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 fix/fully-qualified-symbol-indent branch from 0ae25b4 to baca35b Compare April 5, 2025 08:52
@bbatsov
Copy link
Member

bbatsov commented Apr 5, 2025

Ah, yeah - I had forgotten about this, but it's kind of important, otherwise it's not possible to tell apart indentation metadata for the same symbols in different namespaces.

Probably in clojure-ts-mode it makes sense to assume that if something is passed unqualified we should use the current namespace. On the other hand - I don't think we'll be able to deal with things like ns aliases, at least not without parsing everything in the ns form.

@rrudakov rrudakov force-pushed the fix/fully-qualified-symbol-indent branch from baca35b to 3761b2d Compare April 5, 2025 14:21
@rrudakov
Copy link
Contributor Author

rrudakov commented Apr 5, 2025

Probably in clojure-ts-mode it makes sense to assume that if something is passed unqualified we should use the current namespace. On the other hand - I don't think we'll be able to deal with things like ns aliases, at least not without parsing everything in the ns form.

Hmm, that's a good point, if macro is defined in the same namespace then this function will be called without ns in current implementation, but macro can also be required using :refer and using current ns will be not valid solution.

@rrudakov
Copy link
Contributor Author

rrudakov commented Apr 5, 2025

@bbatsov I'd probably suggest to merge this as is. I'll try to figure out how to deal with edge cases separately.

@rrudakov
Copy link
Contributor Author

rrudakov commented Apr 5, 2025

I've just tested it with CIDER, if a macro is imported using :refer it's actually being successfully resolved by cider-resolve--get-in, it also works fine if macro is imported with :use keyword, so it looks like that the fix from this PR is sufficient to make it work with CIDER.

@bbatsov bbatsov merged commit c213950 into clojure-emacs:main Apr 5, 2025
2 of 3 checks passed
@bbatsov
Copy link
Member

bbatsov commented Apr 5, 2025

Deal!

@rrudakov rrudakov deleted the fix/fully-qualified-symbol-indent branch April 5, 2025 16:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants