clojure-ts-mode icon indicating copy to clipboard operation
clojure-ts-mode copied to clipboard

Support toplevel defs with metadata

Open kommen opened this issue 1 year ago • 2 comments

Towards fixing #42

Pushing this as WIP and to collect feedback from others if this is the right approach.

  • [x] don't indent a toplevel def with metadata
  • [x] font lock toplevel def with metadata
  • [ ] font lock toplevel def symbol name
  • [ ] make toplevel def with metadata findable via imenu
  • [ ] don't indent a toplevel vector, set or map with metadata

kommen avatar Mar 31 '24 21:03 kommen

I guess that's still waiting for feedback, right?

bbatsov avatar Jun 04 '24 07:06 bbatsov

@bbatsov yes, feedback very welcome! however, I'm using this PR in my prod emacs setup for a few months now and it seems to work as intended. So I would be somewhat confident to merge it for now as is and continue with the remaining todos in a follow up, if no one else has objections.

kommen avatar Jun 04 '24 08:06 kommen

Functionality wise this PR now seems complete to me: I think some of the code needs a bit of clarification and better naming.

I will use my recent changes for a while myself in my daily workflow before I will ask for merging this PR. Meanwhile if folks want to give it a try or discuss the changes with me (here or also on the Clojurians slack), I would appreciate this very much.

I'd be also up to split out some changes in their own PR if others are more comfortable to review smaller chunks (ie. font lock fixes, imenu fixes, indentation fixes).

kommen avatar Oct 19 '24 07:10 kommen

I agree it'd better if we split this into several smaller PRs.

bbatsov avatar Oct 19 '24 07:10 bbatsov

@bbatsov agreed. I will start splitting it into separate PRs once I've had a chance to use it a while all together.

kommen avatar Oct 19 '24 08:10 kommen

As discussed, splitted out the first set of changes to fix the imenu integration here: https://github.com/clojure-emacs/clojure-ts-mode/pull/54

kommen avatar Oct 23 '24 19:10 kommen

And the font locking changes here: https://github.com/clojure-emacs/clojure-ts-mode/pull/55

kommen avatar Oct 25 '24 21:10 kommen

Now that #54 and #55 have been merged we can close this PR.

bbatsov avatar Oct 27 '24 04:10 bbatsov

There are still the indentation fixes from this PR which I still will separate out and as with #55 try to port some relevant test utilities over from clojure-mode

kommen avatar Oct 27 '24 07:10 kommen