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.

Sign up for GitHub

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

Jump to bottom

Support toplevel defs with metadata #43

Closed
wants to merge 23 commits into from
Closed

Support toplevel defs with metadata #43

Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9112b4d
Allow font locking of `def`s at the toplevel with metadata
kommen Mar 31, 2024
ce90a9b
Don't indent toplevel `def`s with metadata
kommen Mar 31, 2024
7867ed5
Fix bytecompile
kommen Mar 31, 2024
c956302
Better solution for indentation of forms with meta, add tests
kommen Oct 15, 2024
db53936
Lint
kommen Oct 15, 2024
56fbdbc
Merge remote-tracking branch 'origin/main' into tld-with-metadata
kommen Oct 16, 2024
84a56ce
Correctly font-lock docstring in def/defn with meta data
kommen Oct 18, 2024
32dc24a
Fix body indentation with meta data
kommen Oct 18, 2024
3b2e674
Use clojure-ts--meta-node-p
kommen Oct 18, 2024
702af2e
Fix imenu entries for def/defn having meta data
kommen Oct 18, 2024
026b65d
Refactor using clojure-ts--node-child-skip-meta
kommen Oct 18, 2024
75aa34d
Correctly font lock the name symbols of def and defn with meta data
kommen Oct 18, 2024
75edb95
Lint
kommen Oct 18, 2024
774330d
Use 'metadata' instead of just 'meta' in function names
kommen Oct 19, 2024
cf90573
Small code formatting fix
kommen Oct 19, 2024
6d71712
Touch up clojure-ts--node-child-skip-metadata
kommen Oct 19, 2024
919884b
Remove unused function args
kommen Oct 19, 2024
b1e4ab0
Lint
kommen Oct 19, 2024
997c591
Add tests for the imenu fixes
kommen Oct 20, 2024
7e44026
Replace unneded let*
kommen Oct 20, 2024
9152ab6
Fix call to clojure-ts--match-with-metadata
kommen Oct 21, 2024
d495e11
Try lint
kommen Oct 21, 2024
eafed20
Try lint
kommen Oct 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion clojure-ts-mode.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ with the markdown_inline grammar."

:feature 'builtin
:language 'clojure
`(((list_lit :anchor (sym_lit (sym_name) @font-lock-keyword-face))
`(((list_lit meta: _ :? :anchor (sym_lit (sym_name) @font-lock-keyword-face))
Copy link
Contributor

Choose a reason for hiding this comment

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

This gets you some of the way to syntax highlighting with these types of forms. A similar fix I believe is also needed for just about anything that follows the pattern of matching

(list_lit :anchor (sym_lit (sym_name @capture)  ...) ...)

Because any list literal can have metadata attached to the beginning. A lot of these queries want to capture the first symbol in the list to match special function calls, so they had to be :anchored to the front of the list. They all break down when metadata is present because the syntax tree lists the metadata as the first literal sub element of the list node.

This also applies to the docstring queries in clojure-ts--docstring-query.

(defn a
 "hello" ;; <- highlighted with font-lock-doc-face
  [] "world")

and

^{:cat :dog}
(defn b
 "hello" ;; <- not highlighted
  [] "world")

The "hello" string in both should be highlighted with

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 can't get this to work for now. Shall I revert my change related to this and get the PR merged?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am unsure. I need to set aside some time to get reacquainted with this problem. I'll try to do that within the next week and have a better answer for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dannyfreeman thank you! meanwhile, I had a chance to dive deeper and I think I actually figured it out (mostly?). there are a bunch of similar queries where the the optional metadata node could be added, but I didn't have time to come up with actual clojure code examples testing them so a didn't want to touch them – maybe unnecessarily.

(:match ,clojure-ts--builtin-symbol-regexp @font-lock-keyword-face))
((sym_name) @font-lock-builtin-face
(:match ,clojure-ts--builtin-dynamic-var-regexp @font-lock-builtin-face)))
Expand Down Expand Up @@ -810,6 +810,13 @@ forms like deftype, defrecord, reify, proxy, etc."
(clojure-ts--match-fn-docstring parent)
(clojure-ts--match-method-docstring parent))))

(defun clojure-ts--match-toplevel-with-meta (_node parent _bol)
"Match NODE when it is toplevel form and it has metadata"
(let* ((grandparent (treesit-node-parent parent)))
(and (string-equal "source" (treesit-node-type grandparent))
(clojure-ts--list-node-p parent)
(treesit-node-child-by-field-name parent "meta"))))
Copy link
Contributor

Choose a reason for hiding this comment

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

The indent rules are some of the hardest to get right. The current indent rule that picks up the def is

Matched rule: ((parent-is "list_lit") parent 1

Which I believe is happening because the when the cursor is between the metadata and the def form ^{:a 1}|(def ...) the parent is technically the list.
I think what we want is a rule that comes before (parent-is "list_lit"), which matches when the parent is a list and the first sibling is meatadata, to indent 0. That might need a special function.

Trying to match the source string at the top should not be done though. That will not apply the rule to anything nested

(comment
  ^{:a 1}|(def a 2)
  )

Copy link
Contributor Author

@kommen kommen Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@dannyfreeman thanks. I addressed this and added some test cases and checked that it matches the behavior of clojure-mode on them.


(defun clojure-ts--semantic-indent-rules ()
"Return a list of indentation rules for `treesit-simple-indent-rules'."
`((clojure
Expand All @@ -822,6 +829,7 @@ forms like deftype, defrecord, reify, proxy, etc."
(clojure-ts--match-threading-macro-arg prev-sibling 0)
;; https://guide.clojure.style/#vertically-align-fn-args
(clojure-ts--match-function-call-arg (nth-sibling 2 nil) 0)
(clojure-ts--match-toplevel-with-meta parent 0)
;; Literal Sequences
((parent-is "list_lit") parent 1) ;; https://guide.clojure.style/#one-space-indent
((parent-is "vec_lit") parent 1) ;; https://guide.clojure.style/#bindings-alignment
Expand Down
Loading