Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

feat(search): Add Syntax Highlighting for Hack Language #62770

Merged
merged 57 commits into from
Jun 6, 2024
Merged

Conversation

mmanela
Copy link
Contributor

@mmanela mmanela commented May 17, 2024

As part of GRAPH-205 for support Hack language, this PR adds Syntax highlighting.
Fixes GRAPH-617

Key notes

  1. Added a reference to the Hack tree-sitter package
  2. Add a highlights query for identifying the items to highlight (Note: I built our version off of the solid foundation found in the Neovim tree sitter repo. But iterated a decent amount for how we would like it highlighted.
  3. Added updated test cases trying to cover many interesting languages features. Hack is a large language and I am not covering everything possible but many key features.
  4. It seems there are some gaps in the upstream tree-sitter grammar such as doesn't support module statement and doesn't support string interpolation (parses just as string)

Upstream bugs filed

Screenshots

image

Test plan

  • Unit tests
  • Validate UI and ensure it looks appropriate

@varungandhi-src
Copy link
Contributor

It seems there are some gaps in the upstream tree-sitter grammar such as doesn't support module statement and doesn't support string interpolation (parses just as string)

Please file 1-2 issues upstream so that we can link them here.

@muglug
Copy link

muglug commented May 22, 2024

Thanks for doing this!

I know you have the tree-sitter grammar already, but we also have a slightly-more-up-to-date TextMate grammar here that supports module and more: https://github.com/slackhq/vscode-hack/blob/master/syntaxes/hack.json

@mmanela
Copy link
Contributor Author

mmanela commented May 22, 2024

@muglug Thanks. Our highlighting is based on tree-sitter, so would be great to get the missing parts added to the hack tree-sitter lib. I plan to file issues there on what I found

@muglug
Copy link

muglug commented May 22, 2024

Sounds good — at Slack we’re not yet using modules, so that’s no sort of blocker. Once we start considering that we’ll push necessary changes to the tree-sitter grammar.

@mmanela
Copy link
Contributor Author

mmanela commented May 22, 2024

Yea I think the bigger annoyance will be the lack of string interpolation support

@mmanela mmanela marked this pull request as ready for review May 22, 2024 20:29
@mmanela mmanela requested a review from keynmol May 23, 2024 14:22
Copy link
Contributor

@varungandhi-src varungandhi-src left a comment

Choose a reason for hiding this comment

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

Please go through https://docs.hhvm.com and identify the different syntax forms which aren't being tested and add tests for those. I've identified some which are missing, I haven't tried to be exhaustive.

@mmanela mmanela changed the title feat(graph): Add Syntax Highlighting for Hack Language feat(search): Add Syntax Highlighting for Hack Language Jun 6, 2024
@mmanela mmanela merged commit 82cff73 into main Jun 6, 2024
14 checks passed
@mmanela mmanela deleted the mmanela/GRAPH-205 branch June 6, 2024 20:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants