pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

Another batch of Clojure enhancements

Open mauricioszabo opened this issue 2 years ago • 5 comments

Replaces #663 because this is more complete.

That was a journey, but basically Clojure highlights some interesting things:

  1. Added a different Clojure grammar - https://github.com/mauricioszabo/tree-sitter-clojure - that adds quotes for strings (so we can add injection points on strings)
  2. Highlights some interesting cases like syntax quoting and unquoting (a screenshot with a custom CSS to highlight these scopes): image
  3. Metadata highlighting (again, with custom CSS): image

Missing:

  • [x] Make EDN grammar behave like Clojure (most plug-ins don't work if you don't detect the filetype as Clojure)
  • [x] Check if there are performance issues that can be solved in this new scope resolver
  • [x] Changelog update

mauricioszabo avatar Sep 19 '23 00:09 mauricioszabo

Looks like this is still set up for the legacy tree-sitter implementation, it would need to use the parserSource key in order to be a "new tree-sitter" grammar.

It also looks like this adds some new functions to some base files - won't those potentially break other tree-sitter grammars?

Spiker985 avatar Apr 09 '24 22:04 Spiker985

@Spiker985 there's no legacy Clojure tree-sitter actually, so I'm unsure what you mean with this options.

As for the new options, they are just new things we support, if other grammars don't use these they won't be affected. But before everything, I need to fix these grammars (or even remove what I did) - injection points are not correct, and they are janky when editing these forms...

mauricioszabo avatar Apr 09 '24 22:04 mauricioszabo

Looks like this is still set up for the legacy tree-sitter implementation, it would need to use the parserSource key in order to be a "new tree-sitter" grammar.

parserSource is a key that doesn't have a function right now except to help the user understand where a compiled WASM file comes from, and when it might need updating. That said, it does need to be present in order to make CI happy, which is why the “Validate WASM Grammar PR Changes” check is failing.

savetheclocktower avatar Apr 09 '24 23:04 savetheclocktower

It also looks like this adds some new functions to some base files - won't those potentially break other tree-sitter grammars?

The changes introduced to core don't change how any existing features behave. We've got

  • A new scope test called ancestorTypeNearerThan (which I think I proposed a while back as a way of solving a problem that Clojure had; it makes several different tests of node ancestry, and without something like this, they're bound to conflict with one another unless you can tell which one is the closest ancestor).
  • A new argument to the content callback of atom.grammars.addInjectionPoint, which won't bother anything that isn't expecting a second argument. I didn't dig too deeply into how it's used in the PR because Clojure scares me, but including the Buffer as a second argument is fine with me, and doesn't have any real downsides that I can think of.

If the tests pass, I'm good.

savetheclocktower avatar Apr 09 '24 23:04 savetheclocktower

We can try merging in the macOS CI fix from master branch if wanting a proper CI run for this PR.

DeeDeeG avatar May 08 '24 22:05 DeeDeeG