fix(highlighting): Improve highlighting for Magik language

[mmanela]

Update Magik highlighting to better classify _unset and symbols.

Test plan

• Update test cases
• Visually inspect

Changelog

• Correctly identify null and symbols in Magik language syntax highlighting

[sebastiaanspeck]

_not is missing in the array of keywords

[mmanela]

@sebastiaanspeck This (unary_operator operator: _ @operator) covers the _not operator.

[sebastiaanspeck]

As a general comment; we did not use the highlights.scm ourselves so that is why that was unfinished work in the original repository, but we do use the tree-sitter in Emacs, with a own prog-mode: https://github.com/roadrunner1776/magik/blob/master/magik-treesit.el. Maybe this could help as another guideline to match how we want to show an item

[sebastiaanspeck]

To prepare for the upgrade to the latest tree-sitter, I would suggest to move this array to the top of the file. Since tree-sitter 21.0 they removed the apply-all-captures flag, making last-wins precedence the default.

NOTE: This change might cause breakage in your grammar's highlight tests.
Just flip the order around of the relevant queries, and keep in mind that the
last query that matches will win.

Keeping the current order results into a test failure:

_package user
# <- keyword
#         ^ identifier.module

syntax highlighting:
      ✗ package.magik
      Failure - row: 0, column: 10, expected highlight 'identifier.module', actual highlights: 'variable'

[mmanela]

Thanks for pointing that out, I think we will need to do work across all our highlighting files when we upgrade to this version since I believe many will have issues. I will plan to wait for that change and test them all together

[sebastiaanspeck]

Is there any roadmap for upgrading the tree-sitter? I also wonder if it would be useful to use highlight.scm (and locals.scm etc.) from the original repository. It creates a lot of overhead for this repository and extra work to keep this repository up-to-date if the grammar ever changes

[varungandhi-src]

Is there any roadmap for upgrading the tree-sitter?

Not at the moment.

I also wonder if it would be useful to use highlight.scm (and locals.scm etc.) from the original repository. It creates a lot of overhead for this repository and extra work to keep this repository up-to-date if the grammar ever changes

From Sourcegraph's POV, upstream highlighting queries (in general, not specific to Magik) are neither sufficiently standardized nor always correct (because maybe the maintainer isn't using them, but they were added some some drive-by contributor), so it's best for us to have all the highlighting queries so we can evolve them together.

[sebastiaanspeck]

Clear story, I am planning myself to back-port the highlights.scm after this PR is merged to make it easier for other projects to adopt the original repository instead of writing it again on their own and to make the original repository more complete, with tests

[varungandhi-src]

That's fair. However, not all grammar maintainers are equally responsive (for understandable reasons), and we need a strategy that works generally.

[sebastiaanspeck]

That's fair. However, not all grammar maintainers are equally responsive (for understandable reasons), and we need a strategy that works generally.

We are grateful to make our grammar (and highlights) even better thanks to you. Right now, we are our only users and it is a good test to see if we missed anything by having a third party integrate our grammar. Hopefully the fork you created of our grammar is soon superfluous.