richpl / PyBasic

Simple interactive BASIC interpreter written in Python
GNU General Public License v3.0
165 stars 45 forks source link

Modularize and Abstract print/input #34

Closed brickbots closed 2 years ago

brickbots commented 3 years ago

Okay... so this is not necessarily intended to be merged immediately, more as a discussion starter to get some ideas going around encapsulating PyBasic for use in other projects.

The primary goal here is to allow other projects to include/integrate the PyBasic code without having to modify/monkeypatch it. This will make accepting upstream improvements much easier. I tried to anticipate customization/implementation specific changes (based on my own experience adding it to a handheld system with it's own keyboard/display) and make these possible via subclassing and/or the terminal abstraction.

Here's the top level of what I've done here:

Happy to hear feedback on either the way this is modularized and/or the features that might make sense to implement in the Terminal class. I'll update with a curses based Terminal class that implements the rest of the proposed Terminal features (clear screen, single char input, input polling, specific screen positioning) if people are interested. These would only make sense along with some new basic keywords to make them useful in actual programs :-)

richpl commented 3 years ago

Sounds good in principle @brickbots, let me just get my head around it fully

brickbots commented 3 years ago

Still WIP and not ready to merge even though there are no conflicts.

brickbots commented 3 years ago

An example curses based terminal is working now, and I've added a CLEAR command to clear the screen either at the interpreter prompt or in a program statement.

I'll be implementing a CURSOR row, column command to change where on screen characters are printed and check in an example program next.

brickbots commented 3 years ago

Curses based terminal is fully implemented, and I've included a demo program to show how the screen addressing works.

To run the curses PyBasic version python example_curses.py

If you run examples/cursor_demo.bas it asks for a delay (5000 works pretty well on my machine, lower numbers = faster execution), clears the screen and bounces an * around using cursor and print commands.

This about wraps up my proof of concept. Let me know any thoughts!

If we go with this terminal abstraction system one of the first things I'd like to do is write a 'terminal' that serves as a test harness for regression.bas. Rather than see the whole output of the program, which is hard for a human to spot issues, we can have the harness look for discrepancies in the expected output and return only pass/fail for each test. The raw output from regression would include delimiters indicating test name, expected output, and actual test output i.e.:

* Check nested subroutines
: firstsecond
firstsecond
* Check multi-array DIM on single line
: 23
23
* Check left$
: foo
foobar

* Would indicate the test name : indicates expected value

Since the test harness is seeing all the print outputs, it can store the expected value, compare it with the next line printed, and indicate pass/fail. Even raising an error to stop execution on failure.

Check nested subroutines
-- PASS
Check multi-array DIM on single line
-- PASS
Check left$
-- FAIL, tests halted line 220
brickbots commented 2 years ago

Okay... I think I'm done with this POC :-)

The Curses based terminal is implemented with screen control, and up/down arrow for scroll back. There is a new CLEAR command which can be used directly in the interpreter or as part of a program along with a CURSOR column, row command to place the cursor at a specific spot before printing. There is an example program examples/cursor_demo.bas to show this all working.

If you are on windows, I think there may be an additional non-default package required: python -m pip install windows-curses If this is a direction we want to go, I can look into a more windows friendly option or provide guidance. PyBasic works just fine with the standard simple terminal, but the up/down arrow for scroll back and wider variety of games/interaction with screen control is nice.

This last commit/push implements the test harness. This allows full integrated and potentially automated testing of the interpreter and related systems. I took all the tests in regression.bas and ported them to tests.bas with more structured output that the testing terminal can use. There is a run_test.py script which uses the test terminal class and the program class directly to load and execute tests.bas:

$ python run_tests.py
+++++++++ TESTS STARTING +++++
TEST: Addition / Compound statements
    PASSED
TEST: Multiplication
    PASSED
TEST: Order of operations A
    PASSED
TEST: Order of operations B
    PASSED
TEST:  conditional branching IF THEN
    PASSED
TEST:  Unconditional branching GOTO
    PASSED
TEST:  Gosub
    PASSED
TEST:  Nested gosub
    PASSED
TEST:  FOR loop, simple
    PASSED
TEST:  FOR loop, with negative step
    PASSED
TEST:  Nested loops
    PASSED
TEST:  Array dim/set/get behavior test
    PASSED
TEST:  FILE IO Test
    PASSED
TEST:  DATA Test A
    PASSED
TEST:  DATA Test B
    PASSED
TEST:  DATA Test C
    PASSED
TEST:  DATA Test D
    PASSED
TEST:  DATA Test E
    PASSED
+++++++++ TESTS COMPLETE +++++

This will exit with a ZERO exit code if all tests pass, or will halt and exit with a -1 code if any test fails. This will allow the running of this testing setup each time code is committed to this repo or a PR is generated. If you so choose, @richpl

Let me know any thoughts. If any of the ideas in here are desired, let me know and I'll rebase the so the git history is clean and open another PR 👍

richpl commented 2 years ago

Hey @brickbots, I'm not at all familiar with curses. It seems to involve some pretty pervasive changes across multiple files and I wonder whether this will overcomplicate the codebase somewhat (there is a lot more to get your head around as a developer). I think maybe this is better off as a fork from the main codebase, but it could just be my ignorance talking.

brickbots commented 2 years ago

Hi @richpl! It's definitely a fair bit of change and it's totally understandable that you might not want to head this direction.

PyBasic as it stands right now is clear, easy to grok and works wonderfully as a stand-alone basic interpreter. The changes to get from here, to making PyBasic a module that other people can incorporate gracefully into other projects would definitely have the side affect of complicating things.

Looking forward at some of the other changes that might develop from this direction, it's probably going to get even more complex. Tweaking the codebase to allow layering on new keywords/functions would be a somewhat involved undertaking..... after that is potentially some work to make the whole thing performant on memory constrained hardware... which would definitely make the code less digestible!

If you are okay, I might strike out in this direction on my fork. Hopefully they won't get too far apart that we can't share important changes/bug fixes. Even if the merging get's tricky, we can always hand incorporate specific updates!

Thanks so much for this awesome base and it's been a bunch of fun working with your project. I'll keep an eye on the issue board here to see if there is anything I can chip in on 👍

richpl commented 2 years ago

Okay, thanks for your understanding @brickbots. I think we probably are getting away from the simplicity of the original implementation, and the need for a non-default package to support Windows highlights how the platform dependencies are creeping in. So I'll close this request.

Interested to see how things progress in your fork though. I'm happy to put in an explicit link from my Readme if you'd like. I could also do the same for @RetiredWizard and @JiFish.