robhagemans / pcbasic

PC-BASIC - A free, cross-platform emulator for the GW-BASIC family of interpreters
http://www.pc-basic.org
Other
396 stars 48 forks source link

Remove "just after list" error in ON GOTO/GOSUB #147

Closed rbergen closed 2 years ago

rbergen commented 2 years ago

This fixes #139.

interpreter.py is actually the only file that was changed, but git keeps insisting the .BAS files are modified as well; I'm guessing due to .gitattributes doing its thing to .BAS file line endings.

robhagemans commented 2 years ago

Thanks! I do wonder what I was thinking with the original line - given that there's a comment explicitly stating there should be an error, which is incorrect...

robhagemans commented 2 years ago

I think the case the original line was meant to cover is on i goto 100, 200,,400, where i=3 does give a Syntax error in GW-BASIC:

onerrorgoto

robhagemans commented 2 years ago

As to the BAS-files changing and git stubbornly refusing to revert hem - I've had that before (on Linux) but thought it was resolved (maybe only on develop branch?). The root cause is that theBASfiles have explicit MS-DOS CRLF line endings, whilegit` tends to change line endings depending on the OS's standard. Out of interest, which OS are you on?

rbergen commented 2 years ago

I think the case the original line was meant to cover is on i goto 100, 200,,400, where i=3 does give a Syntax error in GW-BASIC:

I see. I guess that makes sense, but I've just confirmed that the lines I removed are not necessary to achieve this. With the lines removed, a syntax error is still thrown if the line number is missing (because the jumpnum argument to jump or jump_sub is invalid).

Out of interest, which OS are you on?

I'm on Windows 10, doing the work on PC-BASIC under WSL2/Ubuntu. Anyway, I did create the branch for this fix off of develop, so I guess it is an even more intricate issue than I care to understand.

robhagemans commented 2 years ago

The following seems to fix the issue with ghost changes on the files with CRLF endings:

git config --global core.autocrlf false
git rm .gitattributes
git checkout -f .gitattributes

Basically the way git deals with line endings seems just a bit broken and makes my head hurt. There must be a less painful way to do this, but this at least keeps the changes out of the tree...

rbergen commented 2 years ago

I'll try that when I can, later today. I stopped pulling my hair out over it; I don't have that much left to begin with.

robhagemans commented 2 years ago

Merged - @rbergen - to get rid of the .BAS files issue I ended up reconstructing your commit, keeping you as author

rbergen commented 2 years ago

Merged - @rbergen - to get rid of the .BAS files issue I ended up reconstructing your commit, keeping you as author

@robhagemans Thank you for cleaning up my commit for me. I did change git config as you suggested, so I should be setup for any future contributions I might make.

robhagemans commented 2 years ago

Screenshot for the aficionados. So there should be an error, but only if the selector value equals 1.

And this is why building a GW-BASIC emulator is an insane project

Screenshot from 2021-11-06 18-32-47

rbergen commented 2 years ago

What's most worrying for me is that I can actually see why this makes sense, in a GW-BASIC kind of way.

robhagemans commented 2 years ago

Hmm, please explain... I'm mystified. Though not surprised as I have got used to incomprehensible quirks.

Presumably the interpreter has a position pointer which it moves forward, counting commas until the value of A-1 is reached, then executing the GOTO or GOSUB with the line number token found at that position. If no line number token is found, that's a Syntax Error. However, I would expect that to also trigger on the ON 3 GOTO 20, 30 - i.e. the behaviour of PC-BASIC prior to this fix - but clearly it doesn't. So for now I'm stumped, but open to enlightenment :)

Generally quirks like this are very interesting, quite often the behaviour on corner cases and errors like this points to some insight about how GW-BASIC was implemented. Keeping in mind that it was manually coded in assembly and some of its behaviours are actual bugs, but the distinction between bugs and quirks is a fuzzy one.

robhagemans commented 2 years ago

Actually, that's it - commas are the key

ON 2 GOTO 1
Ok
ON 2 GOTO 1,
Syntax error
Ok
robhagemans commented 2 years ago

I think (hope) 104e77fc8 nails it

rbergen commented 2 years ago

It's basically exactly what the comment says that my PR removed: # missing jump *just where we need it* is an error If the ON ... syntax is such that there should be a numeric value exactly at the spot the jump index number points to, then it's a syntax error. ON 1 GOTO is an error because there must be a number immediately after GOTO (jump index 1). ON 2 GOTO 5 is not an error: there is a value for jump index 1 and no "placeholder" for jump index 2, so jump index 2 is a simple "index overshoot", which is OK. That's also why ON 0 GOTO and ON 2 GOTO are OK: those are respectively a jump index undershoot and overshoot, both of which are again OK. ON 2 GOTO 5, is an error, because now there is a "target placeholder" for jump index 2 due to the comma, but no actual value to jump to.

rbergen commented 2 years ago

I think (hope) 104e77f nails it

I won't claim I've come close to testing every possible scenario, but I've made a real effort to check the recognisable variations. They all seem to work fine now in develop.