mvextensions / mvbasic

MultiValue Basic extension for Visual Studio Code
MIT License
30 stars 16 forks source link

[BUG] Leading, valid keywords not recognised in certain scenarios #64

Closed mikes-zum closed 4 years ago

mikes-zum commented 4 years ago

Describe the bug In certain constructs, (leading) valid keywords are not recognised

To Reproduce Steps to reproduce the behavior:

  1. Enter the following code: LOOP READT TapeRec FROM 5 ELSE Reason = SYSTEM(0) IF Reason = 2 THEN BREAK ;* done CRT "Tape error" ; STOP END REPEAT
  2. Click on 'READT', "FROM", "ELSE", IF",CRT"
  3. No intellisense shows, but does for "SYSTEM","BREAK" and "STOP"
  4. The same applies if there is no preceding space, i.e.: PROGRAM doesn't work but PROGRAM (with a preceding space) does

Expected behavior Keywords should be recognised whether they are in a loop, or there is no preceding Screenshots If applicable, add screenshots to help explain your problem.

Versions of applicable software Windows 10 1909 VS Code Insiders 1.42.0-insiders MV Basic 2.0.2 jBASE Language

Code samples and/or reproduceable test cases LOOP READT TapeRec FROM 5 ELSE Reason = SYSTEM(0) IF Reason = 2 THEN BREAK ;* done CRT "Tape error" ; STOP END REPEAT

Additional context Add any other context about the problem here.

ianmcgowan commented 4 years ago

This looks like a mismatch between the MvLanguage.json and jBASELanguage.json intellisense support (READT is in the jBASE file. but not MVON). Are you working with jBASE code?

If you change this line in server.ts from

filePath = filePath + path.join('../','../','../','Syntaxes', 'MvLanguage.json');

To:

filePath = filePath + path.join('../','../','../','Syntaxes', 'jBASELanguage.json');

Then READT gets the expected hover intellisense. I find the formatting/ linting/ intellisense/ snippets quite confusing - there are different files to control each, and different ways of telling VSCode which to use.

image

mikes-zum commented 4 years ago

I was not aware of that setting, but changing it to "jBASELanguage" has no effect.

I am editing the files directly rather than via any sort of connection, so I am not sure that it is relevant in this case, plus most things have worked fine so far, but this is one that I have been encountering for some time, both in the release version and the insiders version.

ianmcgowan commented 4 years ago

Yep, changing it directly won't work unless you're running the extension in debug mode. There's something coming to enable dynamically switching flavor/syntax, but in the meantime it seems like READT is valid syntax is almost all flavors, so adding it to the intellisense and format configs seems to make sense. That will mean this isn't fixed until the next release though...

ianmcgowan commented 4 years ago

It looks like https://github.com/mvextensions/mvbasic/blob/master/Syntaxes/MvLanguage.json is truncated - only 553 lines, vs thousands in the other three. It may be that something went wrong when that syntax file was being generated, @GrantHart ?

itsxallwater commented 4 years ago

If memory serves it has always been lighter than the others (could be an opportunity for expansion). I will say, if you happened to check the original branch you might've been left wondering where the heck the file was at the beginning. Initially, Grant had the definition inline in the server.ts file and early on in this project, he extracted it out into the json file that exists today to set the table for the dynamic switching you alluded to. Figured I'd put pen to paper on that since it took me a minute to recall that move when I was first checking the old branch.

kpowick commented 4 years ago

I've made minor update in PR #94 that corrects indentation and syntax highlighting for this issue, but does not (yet) address the missing Intellisense.

kpowick commented 4 years ago

I'll grab this assignment. I'm working on it and would like to have it included in PR #94 for v2.0.6

kpowick commented 4 years ago

This is a strange one. Using the keyword CRT as an example, Intellisense on hover only seems to work when the keyword is within a block as created by a structure like IF/THEN, LOOP/REPEAT, etc.

Enter the sample code below into the editor, then hover your mouse pointer over the occurrences of CRT. You'll only get intellisense feedback where indicated.

CRT 'Intellisense FAIL'

IF A=1 THEN
   CRT 'Intellisense OK'
END

LOOP
   CRT 'Intellisense OK'
REPEAT

CRT 'Intellisense FAIL'

Before I try to dig through the code, do you have any insights @itsxallwater?

itsxallwater commented 4 years ago

You're in luck, I noticed this while I was testing the package updates, clean up and bundling and I just about have a fix ready. Will include this sample code in the testing I'm doing.

mikes-zum commented 4 years ago

If there is a leading space, then it should work. For instance if you use the BI option to format your code in jBASE, then you will not normally encounter this, But if you keep your code flush to the left margin, then you will certainly encounter it. I should also add that keywords in comments also generate "Intellisense"

itsxallwater commented 4 years ago

Yes indeed, essentially had to do with some bad index tracking while it was parsing a line to determine the word being hovered. If the line started at index 0 it was chopping the first character off, so those CRT lookups were becoming RT. I've got that fixed just checking on a few other things before I commit into https://github.com/mvextensions/mvbasic/pull/94

kpowick commented 4 years ago

D'oh! Should have realized it was something simpler like a leading space, instead of being inside a block. Thanks @mikes-zum @itsxallwater