Propose fix for syntax highlighting for some keywords

[bkats]

Some keyword aren't highlighted correctly when at a start of the line. (if, call, jump, ...)
Also a space after '$' was for some reason expected, I remove that check also. One-line python as follows is correct "$test = True", but not recognized by the syntax highlighting.

I have a fix for it, but not sure how to make a proper pull request. I tried but I think I made a mistake in how a cloned the repository? My first time to that I try to do this. So I closed the failed pull-request attempt, not sure how to proceed.
Sorry for this but I saw to late that I needed to make to pull-request to develop and when just did try to make to that branch it added other (unwanted?) commits to the request.

My clone is here: https://github.com/bkats/vscode-language-renpy/tree/fix-spaces

[duckdoom4]

Hi! Thanks for your efforts into fixing this issue.

We welcome your PR with open arms, any help is very welcome at this point ❤️.

So what it looks like is that you need to rebase your branch. If you're using GitHub Desktop, just have your branch selected, then select the option 'rebase branch' and pick upstream/develop. Then use a force push to update your branch to be based on develop.

Warning: If it tells you there's a merge conflict, then just use the 'merge branch into..' option instead.

Let me know how it goes, I'm happy to help out if you get stuck on something

[bkats]

Thanks for the response. I tried a few things, but it didn't work out for me. (Github still didn't understand it, probably because of only forking master)
So I choose the quickest way, just delete the fork and created a new fork, but now forking all branches instead of only master. Redid changes (copy file from backup location) and create a new pull request.
It seems I can't connect pull request to an issue, based on the Github help page. Since it state keyword work only on master branch requests (I didn't try) or using development option, but I guess that only is available to maintainers of this repository. (I don't see that icon there)

I will try to look into other issues I see syntax highlighting, but they are probably more complex. So not sure if I can solve them.

[bkats]

I looked over some of things I noticed with 2.4.0 release in vscode. Most seems to be already fixed here.
But I'm left with one (at the moment):

transform test:
    size (100,100)

Here "size" is colored red, but it is a deprecated property so not sure if this should be fixed or left this way?

Some context from how I use the syntax highlighting:
I made a viewer tool (https://gitlab.com/bkats/renpygameviewer) where I use the json files from this vscode plugin for syntax highlighting. So viewing VNs in the wild made me notice some of the issues. So fixing them for my tool is helping the plugin. But of course the goal might be different, like the "size" coloring.

[duckdoom4]

I think the size coloring should be fixed. Even though it's deprecated, it can still be in codebases and should then be marked as deprecated by the plugin.

[bkats]

I will (try to) fix that or those. The other deprecated properties are probably also missing.

[bkats]

Okay, after quick look.
There are 4 deprecated properties: crop_relative, alignaround, size and maxsize.
Two (crop_relative, alignaround) are just there. (Also can't find when they became deprecated, yet)
The other two (size, maxsize) are specific inside a deprecated block.

So based on your comment about deprecated should be marked by plugin and not syntax highlighter.
I would propose that I change the name field from "invalid.deprecated.renpy" to "entity.other.attribute-name.atl.renpy support.type.property-name.atl.renpy entity.name.tag.css.atl.renpy"
Also maybe add "deprecated" to the comment field?

[duckdoom4]

Sounds good to me!
When you've made a PR, could you verify that those properties get marked as deprecated by the plugin ? (Aka. they are marked with the yellow underline).

If they are not, please let me know and I'll take a look as to why

[duckdoom4]

While we're on the topic of fixing syntax highlighting issues, have you noticed anything else that's missing or broken?

I think I had implemented the last missing feature in the previous update, but I'm not sure

[bkats]

Sounds good to me! When you've made a PR, could you verify that those properties get marked as deprecated by the plugin ? (Aka. they are marked with the yellow underline).

If they are not, please let me know and I'll take a look as to why

So I have change the json file. But hadn't thought about the deprecate marking. But after trying to understand other parts of the extension. It seems you do something inside diagnostics.ts, but how it is done there it wouldn't work for "size". The problem is you just check per line if something is incorrect without any further context. Since "size" is valid inside a screen, it would mark those also obsolete/deprecated.

I assume, didn't completely look into that part, that you are implementing the parser to improve on that? Also not sure what the tokenizer part is for.

But to keep it short at the moment size, maxsize and other two aren't marked deprecated. As far I can tell, since I have only v2.4 active in vscode and don't know to test/build current state of the extension in vscode. (I test the json files in my own written application using TextMateSharp)

While we're on the topic of fixing syntax highlighting issues, have you noticed anything else that's missing or broken?

I think I had implemented the last missing feature in the previous update, but I'm not sure

Yes, I have seen one other styling problem, but didn't mention it yet. Want to focus one thing at the time.
When the value part of a property on a style has a expression like follows: xalign test * 2 then the last part isn't correctly recognized. Same does work in a screen. I haven't dived deep into it, but I noticed that screen has own definition for expressions so it seems that one is more complete? My first thought about it is that I wouldn't expect two definitions for expressions, since they should all be the same? But again have looked any further into it.

[duckdoom4]

I assume, didn't completely look into that part, that you are implementing the parser to improve on that?

Yes, that's exactly why I built the parser implementation :)

Also not sure what the tokenizer part is for.

That's the 'lexer' if that means anything to you. Basically it's the part of the parser that converts text into numbered enum + range data 'tokens'. Eg.

transform test:
   size (100,100)

Would be

[KeywordToken.Transform, EntityToken.Identifier, CharacterToken.NewLine, ...]

Basically it's the same as the values inside the json files, but then on the typescript side.

...how to test/build current state of the extension in vscode

It's very simple, I've setup a full launch environment to make it easy to run. All you do is clone the project, then open the cloned folder in vscode. Then you just press the 'start debug' button in vscode's debugger tab.
(Maybe, you have to run npm install in the folder before it runs, not sure if I had set that up)

Yes, I have seen one other styling problem, but didn't mention it yet. Want to focus one thing at the time.

Np, we can look at it one at a time then :)

.. style has an expression like follows..

Ah, I'm guessing that I missed something in the style json that I did add to the screen json. The style syntax highlighting was added last, so makes sense that something is still missing there. Probably just an include that is a incorrect/missing

[bkats]

Okay, I lost in a rabbit-hole called expression(s). Not sure but there are lot of expressions, but I'm not sure why there are so many variants of this. So I'm not sure how to tackle these, or why there is made so many different ones. There is probably a reason for, but for me hard to tell. So I'm not sure what best option would be.

But on the other side I noticed that the "jump" statement was missing the "pass" option. So I at least fixed that one.

Also I found the launch option. But not sure it works for everything, since I don't have npm (node.js) installed. I get an error when launching that npm watch run fails, but there is a new instance of vscode opened with the changes in syntax highlighting working. At least that is something, too.

[duckdoom4]

Ah yeah, you do need node.js for it to work.

As for the expressions, not sure what you mean. Are you looking at the python tmlang file? Cus that one is directly imported from the python vscode extension (with minor modifications)