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

@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 withentity.name.function.

I will probably need to look into those changes once again, because since I open this PR,@bobzhang was able to shippattern 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

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. Thecomment scope is misleading as well; as you've seen in that post, our priority is correctness of highlighting, so assigningcomment 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.

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?

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 thefriends andname 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! 😅

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

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 asvariable.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 scopesvariable.other.enummember (kind of correct from language point of view) andconstant.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 scopeconstant.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

@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 delightfulhttps://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

What aboutlet with destructuring? I got a special style forlet bindings withOCaml Platform extension, but e.g., identifiers inlet (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.

@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 iswhat 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.

@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.

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.

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 !

@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 :)