Add type, external, let word coloring && unit as constant

[Faliszek]

Hi,

First things first. I want to thank you all for your work on this amazing project and this extension.

And now the rest.

Description

One thing that i want to point straight away, I rarely wrote any regexp so sorry in advance if those changes are just garbage 😅
But I look through on files in my project and I think i didn't break any working grammar.

Presented code is mostly from rescript/reason docs, I think i don't miss any possible scenario for using let, external, and type.

What changed

When working with .re files and with vscode-ocaml-platform I have (imo) beatiful syntax highlighting:

and rescript file look like this:

This pr is mostly about 2 changes:

1. Coloring word after let, type, externalbut also type rec, let rec keyword as entity.name.function (the same way vscode-ocaml-platform does it).
   The word I am matching is [a-z][a-z_A-Z_0-9_]* - this (i hope) match every valid name for type and let name declaration.

NOTE: one small change that I thought would be nice to have is match variable started with _ as comment (line 22 on screens) it looks very cool in my current One Dark Pro theme. (the same way as ocaml-platform)

2. Coloring unit - () - (again inspired by vscode-ocaml-platform)

In my actual theme One Dark Pro

Before

After

Dark+

Before:

After:

[chenglou]

Hey thanks! I'll review this later but make sure you've seen #8 (comment)

[Faliszek]

@chenglou Yep, I saw it and I tried to implement those changes in the smallest way possible, but i have no idea to which scopes we want/should assign those so I went with entity.name.function.

I will probably need to look into those changes once again, because since I open this PR, @bobzhang was able to ship pattern match over modules (which is great)! 🥳 So my PR needs some work. But not sure how we should approach that here (like i said earlier, regexp is not my favorite thing in the world 😄 ). Anyway, any feedback is welcome

[chenglou]

I don't think () and and _unused should be specially highlighted; the former because it feels a bit too specialized and the latter because _unused can still be used, so it'll be misleading. The comment scope is misleading as well; as you've seen in that post, our priority is correctness of highlighting, so assigning comment to _unused kinda doubly violates that.

However it's a good idea to assign some scope to _, because that one's truly "unused"/unusable. There's no good textmate scope for it though...

And yeah this implementation is nice and simple but I think we're missing patterns (each let binding is a pattern too), e.g. let Coords(x) = c and/or fields, but maybe that's ok... It's hard to tell where to cut and where to stop here.

As for types, I'm 1. not sure they should be highlighted the same way as values and 2. Either way they're not recursively highlighted here within fields and for annotations like (a: int). I'm also thinking maybe we should just highlight every lower case identifier, but that might be too much.

And yeah no worries about the regexes. The problem isn't the regex, it's the archaic TextMate grammar format. ReScript's grammar is complex and we're essentially trying to use regexes and very weak TextMate matching logic here to emulate a full blown parser program. That's why in that thread I advised thinking only in tokenizer and not parser (but even that falls apart because JSX and template literal are that complex).

Anyway, just some brain dump.

[Faliszek]

My bad, I tried to also include some jsx highlight - but i just gave up, and forget to remove it 😅

[bjornj12]

What is the status of this PR? would love syntax highlighting on let 🙏

Also is it just me or does this feel a bit weird?
I might be too used to Reasonml syntax (still stuck there working on jsx3 🙈) but should the "name" and "friends" not be a specific color?

[Faliszek]

Hi,
Sorry I am just stuck with work probably until end of May, so I will not continue working on it right now, but I would also love to have the friends and name highlighted, but it is more complicated, and I didnt want to bloat this code unnecessary.

I will try to look at it in June. But I cant make any promises! 😅

[TheSpyder]

This will fix #89 (I guess I didn't search well enough when I logged it)

[Faliszek]

And yeah no worries about the regexes. The problem isn't the regex, it's the archaic TextMate grammar format.

Now I exactly understand what you meant 😄
You guys already did great job with this grammar, but to finish all edge cases even only in all listed themes in README.md with correct scopes from language point of view might be very difficult to get it right.

My latetest changes:

1. Set object property as variable.object.property, but I don't go too far with it. So unfortunately in (a: int) - a also will be highlighted.

But I also wanted to improve the rest of coloring, but even If i find matching scope for something, it is supported in one theme, but not in other. So the next ones are kind of a stretch 😅

2. Add to existing polymorphic variant regexp additional scopes variable.other.enummember (kind of correct from language point of view) and constant.other.symbol (just to have different color) in One Dark pro.
3. I also wanted to improve punctuation, but Dark+ don't highlight this scope. I found a scope constant.character - and it looks nicely in Dark+ and One Dark Pro but once again. Kind of stretch from logical point of view. I added it because when reading code, characters lik (, {, [ - are suggesting for me some kind of scope, and it is just easier to read. I also wanted to highlight "<", ">" but there is some trickery with jsx

Results:
Dark+

One Dark pro

@chenglou No hurt feelings, if this will be closed and not merged. I see now that creating correct grammar with proper logical text-mate scoping and correctnes of colors in popular themes, is a hell of a task to get it right.

But personally I will prefer having not that correct coloring with stupid scopes, than no coloring at all. But I read your reasoning behind it, and I understand you 100%. So if i need to develop with rescript again, i will probably copy my changes and paste it directly to extension grammar :P

Feedback welcome

[Faliszek]

@chenglou Also I am starting to wonder, if dedicated theme for rescript might be a good idea? (for example with two variants dark, and light)

I started play with Elixir, and they are also have complicated grammar, and I find elixir specific theme for vscode really delightful https://marketplace.visualstudio.com/items?itemName=Arsen.darcula-theme-for-elixir

I am might be able to give it a shoot to create one with your guidance, on what, and how we want to highlight things

[hoichi]

What about let with destructuring? I got a special style for let bindings with OCaml Platform extension, but e.g., identifiers in let (state, setState) = React.useState(...) are not tokenized and therefore not highlighted, which is kind of a bummer.

Disclaimer: no idea if TextMate grammar can do it at all.

[TheSpyder]

@Faliszek would you consider releasing this as a standalone vscode extension? Or would you mind if I did one based on your work? I didn't realise a grammar-only extension was possible, but I just found this
https://marketplace.visualstudio.com/items?itemName=glennsl.sane-reason-grammar

A dedicated extension would be easier to use than patching the installed ReScript extension, which is what was recommended on the forum. It would also give us a good place to battle-test highlighting ideas before recommending them to the main extension.

[Faliszek]

@TheSpyder I mean if other authors of this extension have nothing against fork this extension, feel free to use my changes as you like :)

Personally I probably won't find time needed to release and maintain extension in near future.

[TheSpyder]

My intention is not to fork the whole extension and compete, only to have a way to use your grammar without patching installed extensions. I’ll see what I can cook up this weekend.

[zth]

Just dropping a note in here that #367 will implement the equivalent of this, although in a different way. Thank you for your great work @Faliszek !

[Faliszek]

@zth yup, I already saw your PR, it looks great! Thank you so much for doing this! You rock :)

So I think it's ok to close this one :)