-
Notifications
You must be signed in to change notification settings - Fork 22
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
Token highlighting issues #431
Comments
Hi, thanks a lot for the very detailed bug report! I'll be fixing many of these.
The text tags will need to wait. The previous version had a major flaw that I was not able to fix at the time. I might give it another go, but it might not be possible yet. All of that said, the theme you are using is hiding some of the colorization of certain tokens. Eg. The style properties should be a different color if you use the default theme. And the string/text tags should have a different color for the brackets. It might be possible to add some additional tokens to support other themes as well, so might take a look at that to fix this issue for your theme. |
Understood about the theme - I should have mentioned probably, but the theme in the screenshots is Monokai. That said, it'd be great if it could be updated to use some other style set which didn't conflict with the other common tokens. Would you be able to elaborate on the issues you are running into with the text tags? I have very little familiarity with language extensions for VS Code, but I'm good with regex and I've been coding for over a decade, so I'm considering putting some legwork in to learning the ins and outs to be able to pitch in, at least to add new keywords as they get added to the engine. |
Yeah, the issue with the string tags is related to this issue #255. Basically, I used to test for begin to end, where end would test for a closing tag. But it turns out it is allowed to not close the tags as well, in which case it doesn't end until the string end. (I removed it in this commit #330) IIRC, the issue was that I wasn't able to code that behavior into the regex because I would need to keep track of the number of opens and closes. You can find the syntax highlighting rules here https://github.com/renpy/vscode-language-renpy/tree/develop/syntaxes. I'm also working on a parser on a different branch to support semantic highlighting. (Eg. highlighting of variables) |
I've added most of the fixes I mentioned before with #432. Also feel free to contribute any changes you'd like to introduce. I'd be very happy if I could get some help with this project :) |
As of v2.0.16 of the original extension, there are numerous issues with token highlighting, particularly with screens, styles, and text tags. Here are several comparisons: the first image is the highlighting from 2.0.16, and the second is the highlighting from 2.4.0 (current version as of this moment). Then I'll post the actual text used for the examples for convenience.
Dialogue & Script
Issues:
Screens
Issues:
predict
,substitute
,fixed
,yfit
,text_color
label
has inconsistent highlighting (sometimes it appears to be recognized as though it is a script label)Issues:
imagebutton
properties exceptaction
, alltext
properties regardless of whether they're prefixed withtext_
or not,yinitial
,side_
prefixed properties likeside_yfill
,null height 5
vbox
,textbutton
,viewport
should not be the same colour as screen language properties (pos
,yoffset
,mousewheel
etc.). That colour is already shared by many other grammar tokens likeif
,not
,and
, etc. and it becomes oversaturatedPython
Issues
Conclusion
Personally, I find the inconsistency and reused token colour introduces enough visual confusion that I can't use the extension in its current state. The commit 2f508c0 (found here) begins to introduce the token highlighting problem. It may be worthwhile to revert back to the commit where the tokens were working properly and work forwards to introduce any later features while checking for highlighting inconsistencies. I suggest these examples be worked into the test code files as well - there seems to be a lack of representation for screen language and text tags in particular in testing.
The text was updated successfully, but these errors were encountered: