-
-
Notifications
You must be signed in to change notification settings - Fork 69
feat(syntax): add support for Haskell (#360) #559
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
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Sorry for the wait on this @carlwr, I wanted to get around to reviewing this soon but I've pre-occupied with other parts of the organisation in my spare time. Will try to get to this next weekend (23rd/24th Aug) ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to raise this PR!
I've tested it out locally and it does look much better. One change I made was to remove rosewater
imports and leave it to the default.
I tried making some other changes to better align with the style guide but those ended up having side effects that made the theme look worse overall, so happy to approve and merge this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realised that we won't be able to merge without some more changes, sorry!
The removed scopes were redundant since they are not among the scopes assigned by the only in-use Haskell textMate grammar (https://github.com/JustusAdam/language-haskell). The tokens/haskell.ts rules named "typclasses" and "type families" each relied on only such a scope and these rules are therefore removed from the file. Styling of typeclasses and type families is still possible through semantic tokens though.
@sgoudham thanks for review+feedback - just pushed an update. Changes include making all textMate selectors Haskell-specific - I had obviously totally missed that in the original PR. There are also a number of other improvements. In my view the result is now considerably better, and (I think) more aligned to the style guide and de-facto styling of other languages. The above is with The Haskell textMate grammar is unfortunately rather low on semantic information, and the LSP tokens are coarse. Taken together, that explains the somewhat lengthy styling logic, but something like this is seemingly needed for a decent result. Just tell me if you want me to change anything! |
@sgoudham just reminding - also please tell me if there is anything I can do on my part to move this forward. |
So sorry for the wait on this PR, I'll take a look this upcoming weekend @carlwr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this, think it looks much better.
I just had one comment surrounding preprocessers. I think the directives should be rosewater
and I'm not too sure about the red/peach in the following screenshot:

I think it would make sense to make those rosewater/mauve like below:

I'd be keen to hear your thoughts on this as I don't write Haskell myself that much!
Thanks! C-style preprocessorAgree; updated. PragmasTotally; that red + peach doesn't look too good. I've changed the pragma specifier (= capiatlized first word) to For These are colors that could perhaps be considered, and possible reasoning for the choice. Ranked by my own preference:
I pushed |
Considerably improves the colorization of Haskell code.
Notes:
Suitable Haskell sample code to check how this PR colors, and the code used for the screenshots:
https://github.com/catppuccin/catppuccin/blob/main/samples/haskell.hs
Screenshots