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.

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

Upgrade and reenable tree-sitter-haskell #1417

Merged
merged 3 commits into from Jan 8, 2022

Conversation

414owen
Copy link
Contributor

@414owen 414owen commented Jan 1, 2022

I've made changes to tree-sitter-haskell that remove the need for c++14. It now happily compiles with c++11.
I think this is ubiquitous enough to reenable it by default?

@414owen 414owen force-pushed the upgrade-tree-sitter-haskell branch from cb08936 to c8a63a8 Compare January 1, 2022 20:19
@kirawi
Copy link
Member

kirawi commented Jan 2, 2022

Looks like you'll have to enable C++11 in build_library().

@archseer
Copy link
Member

archseer commented Jan 2, 2022

if cfg!(windows) {

Should look something like 212f6bc

@kirawi
Copy link
Member

kirawi commented Jan 2, 2022

I see you also have a PR to drop the requirement to C++03: 414owen/tree-sitter-haskell#1
I'm not sure, but I think you wouldn't have to change anything else in Helix if that were to get merged,

@414owen
Copy link
Contributor Author

414owen commented Jan 2, 2022

I see you also have a PR to drop the requirement to C++03: 414owen/tree-sitter-haskell#1 I'm not sure, but I think you wouldn't have to change anything else in Helix if that were to get merged,

I'll see if I can get that touched up.

@414owen
Copy link
Contributor Author

414owen commented Jan 3, 2022

Okay I've changed tree-sitter-haskell to only rely on a c99 compiler. Should be portable enough :)

@archseer archseer linked an issue Jan 4, 2022 that may be closed by this pull request
@archseer
Copy link
Member

archseer commented Jan 4, 2022

This is great!

You will probably also want to pull in the locals.scm query and update the highlights.scm one: https://github.com/tree-sitter/tree-sitter-haskell/tree/master/queries

The current file predates the tree-sitter-haskell rewrite.

@414owen 414owen force-pushed the upgrade-tree-sitter-haskell branch from dc57ade to d839931 Compare January 4, 2022 12:05
@414owen
Copy link
Contributor Author

414owen commented Jan 5, 2022

@archseer Looks like those files were updated 19 days ago and 7 months ago, whereas the haskell-tree-sitter grammar's last major features were added in March 2021. I think this should be okay?

It certainly seems to be highlighting correctly when I test it.

If not, what's the process here? Cross-reference helix's known token categories with what haskell-tree-sitter produces?

@kirawi
Copy link
Member

kirawi commented Jan 5, 2022

You should be able to copy the ones in the repo, and then what you'll have to do is change the captures (e.g. @number) to the ones used by Helix (e.g. @constant.numeric.integer).

@414owen
Copy link
Contributor Author

414owen commented Jan 5, 2022

Okay everything in highlights.scm is now understood by helix.

@414owen
Copy link
Contributor Author

414owen commented Jan 5, 2022

The worst mapping (preserved from before) is:

[
  "forall"
  "∀"
] @keyword.control.repeat

In haskell these are quantifiers, and have nothing to do with loops. I don't suppose helix wants to add a keyword.quantifier scope?

@archseer
Copy link
Member

archseer commented Jan 6, 2022

Do you want to style these separately? If not, it's enough to use @keyword

runtime/queries/haskell/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/haskell/highlights.scm Outdated Show resolved Hide resolved
@414owen 414owen force-pushed the upgrade-tree-sitter-haskell branch from a1c52c1 to 54cb214 Compare January 6, 2022 11:13
@414owen
Copy link
Contributor Author

414owen commented Jan 6, 2022

Do you want to style these separately? If not, it's enough to use @keyword

No that's okay. I've changed them to keyword.

@414owen
Copy link
Contributor Author

414owen commented Jan 6, 2022

One last update to highlights.scm to add infix operators (recently added to tree-sitter-haskell).

Think we're ready to merge?

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! 🎉

@archseer archseer merged commit c238f20 into helix-editor:master Jan 8, 2022
heyakyra pushed a commit to heyakyra/helix that referenced this pull request Jan 22, 2022
After the changes to upgrade and reenable tree-sitter-haskell helix-editor#1417
for the purpose of enabling Haskell syntax highlighting helix-editor#1384, we
might as well take the final step.
heyakyra pushed a commit to heyakyra/helix that referenced this pull request Jan 22, 2022
After the changes to upgrade and reenable tree-sitter-haskell helix-editor#1417
for the purpose of enabling Haskell syntax highlighting helix-editor#1384, we
might as well take the final step.
archseer pushed a commit that referenced this pull request Jan 23, 2022
…#1556)

After the changes to upgrade and reenable tree-sitter-haskell #1417
for the purpose of enabling Haskell syntax highlighting #1384, we
might as well take the final step.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Haskell syntax highlighting?
3 participants