TIny-Hacker / language-ti-basic

VS Code language support for (e)Z80 TI-BASIC. Also used by github-linguist
BSD 3-Clause "New" or "Revised" License
14 stars 1 forks source link

Improve syntax highlighting #3

Closed nineteendo closed 8 months ago

nineteendo commented 10 months ago

The extension doesn't handle my byte optimised program very well:

Screenshot 2023-12-03 at 11 03 19

Easy problems:

Harder problems:

TIny-Hacker commented 10 months ago

Thanks! I'll try to look into this.

Variables preceded by are not highlighted because the extension follows the TI-BASIC -> text scheme SourceCoder3 uses, which uses |L instead. Some of those other things should probably be recognized so I'll have to figure out what's going wrong.

nineteendo commented 10 months ago

OK, I've converted the program using SourceCoder3, and indented it properly (the End on line 22 acts like a continue).

nineteendo commented 10 months ago

I found the solution to the easy problems.

TIny-Hacker commented 9 months ago

I've implemented your changes in c7aca74027bd62bbede0948de7be9fe3e767cc80. I've published the update on the Marketplace and the Releases tab as well.

I've been a bit busy this past week but I'll try to look into the other two things. I guess it's time to figure out Regex again :laughing:

nineteendo commented 9 months ago

It looks like you need keyword.operator.expression.8xp instead of keyword.operator.logical.8xp:

keyword.operator.logical is not in the list.

TIny-Hacker commented 9 months ago

As far as I'm aware, how things are highlighted depends on the theme. If a theme doesn't have a color for keyword.operator.logical, it won't be highlighted any differently. IDK if the best move is to keep changing it until it's something that every theme uses, or keep it more with the correct category and then end up with it only being highlighted in some themes.

nineteendo commented 9 months ago

Yes, you're right, that's controlled by the theme. The reason they aren't highlighted right now is because in the default themes keyword.operator overrides the color of keyword with black. Which isn't overridden again, because it doesn't belong to the previously mentioned categories.

To fix this you can classify them as keyword.operator.expression. You don't need to make a release just to fix this.

TIny-Hacker commented 8 months ago

I've looked into the two more difficult issues again and I'm wondering if the best course of action is to just match the token rather than also checking for correct syntax. This is what TI-Planet's Project Builder does, for example. What would you think of that solution?

To clarify: Currently stuff is matched with word breaks for more correct syntax, but removing those would properly highlight the token at all times. However, there's less error checking because checking for just If means something like abc123If would still recognize If.

nineteendo commented 8 months ago

Sounds good, just make sure to match the tokens in the correct order and use lookbehind and lookahead:

Good luck!

Edit: Escaping parenthesis is possible: ANOVA\(. is A * N * O * V * A * (... Edit 2: You're also not detecting matrices yet:

TIny-Hacker commented 8 months ago

I've made the changes and pushed them, along with updating the latest release. Let me know if you feel like the issue has been resolved or there's still changes to make :)

nineteendo commented 8 months ago

AMAZING work, just some minor polishing...

Note that this is NOT a complete list.

TIny-Hacker commented 8 months ago

I've updated the extension and did a little testing of my own and it seems that things are properly detected, at least the way I think they should.

Matr>list( isn't detected because the extension expects Matr▶list, and L1 isn't detected as list 1 because |L1 is.

nineteendo commented 8 months ago

I'm afraid we're not done yet. I've spent the entire day creating a file with every token and they aren't properly highlighted:

Screenshot 2024-01-24 at 16 19 45

Persisting problems (besides the final boss):

I believe that Source Coder 3 does something wrong with the accented letters, so you don't have to worry about that.

It also can't load this file even though TI Connect can.

TIny-Hacker commented 8 months ago

Thanks so much for taking the time to create that file! I found it very useful for debugging highlighting. Have a look at d05c76d01eb627b8336a71dabec6522148343d2e and let me know what you think. There are a few tokens that were not properly highlighted because they were missing a ( or ` at the end, but I've updated the [tokens file](https://github.com/TIny-Hacker/language-ti-basic/files/14055829/tokens2.txt) accordingly to match the way they are shown in TI-OS (I think it was onlyExecLib andgetDtStr(` but there may have been a few more.)

Let me know what you think and when it's ready I'll publish a new release!

nineteendo commented 8 months ago

Looks fairly good, but 3 remarks:

Hmm, are these missing characters getting fixed for Source Coder 3? Because it would be a pain to do manually.

TIny-Hacker commented 8 months ago

The changes are made! I'll try to look into getting it corrected in SourceCoder and confirm what's going on before I publish a release.

nineteendo commented 8 months ago

Hmm, (?<=(Goto) )|(?<=(Lbl) )[0-9A-Zθ]{1,2} is now detecting Goto without a label. I just wanted to make note of the issue with Menu(.

TIny-Hacker commented 8 months ago

Looks like the issue with SourceCoder has been corrected: http://ceme.tech/p305502

I think I need to change the Goto detection to ((?<=(Goto) )|(?<=(Lbl) ))[0-9A-Zθ]{1,2} instead.

nineteendo commented 8 months ago

Seq seems to be fixed, but still 6 differences remain.

Some more ideas:

TIny-Hacker commented 8 months ago

I feel like this is in a good enough state that I'm going to mark this closed for now (unless there's any significant issues left). I'm also considering changing token syntax slightly in #4. Feel free to make a new issue or submit a PR if there's any major issues you're still encountering, but looking through the tokens file you provided everything looks okay to me.

nineteendo commented 8 months ago

I've updated the TOKENS2.8xp.zip.