skx / gobasic

A BASIC interpreter written in golang.
https://blog.steve.fi/tags/basic/
GNU General Public License v2.0
323 stars 27 forks source link

Use output stream prompt #100

Closed andydotxyz closed 5 years ago

andydotxyz commented 5 years ago

Replaces PR #99

andydotxyz commented 5 years ago

Sorry this looks huge as I had to reverse a bunch of local stuff. If you can do a Squash and Merge it should be just what you were looking for.

Thanks

skx commented 5 years ago

Thanks for your contribution, once again.

Is there anything else you have pending, for your emulator project? If not I'll make a new release before the weekend.

I see from testing that I need to remove your addition of the newline in the PRINT function, but otherwise things seem to work well between this and the previous pull-request. (To be honest I could probably drop DUMP as a function, as that was for my own use and was never documented. But if it is useful I guess it could remain.)

Without this removal:

out.WriteRune('\n')

The output from our example-programs are all doubled:

  frodo ~/go/src/github.com/skx/gobasic $ go build . && ./gobasic ./examples/00-print.bas 
  Hello, world

  The contents of the variable 'a' are 3 

  The contents of the variable 'a$' are My name is String 

  String 'Steve' is  5 characters long

  'Steve' is STILL  5 characters long

  'Steve' is STILL  5 characters long
andydotxyz commented 5 years ago

Thanks. That is all I have pending - I want to add “AUTO” support for line numbering but I suspect that will be in my emulator not this library.

DUMP is not something I am using and could be confused with *DUMP.

PRINT is complex. In my emulator the new line is quite important and, as far as I can tell to print without a new line should use PRINT “Inline”; (I.e. trailing semi-colon). http://www.riscos.com/support/developers/bbcbasic/part2/outputting.html

Happy to discuss further

You can run the UI that I’ve been building at https://github.com/andydotxyz/beebui/

skx commented 5 years ago

The download from the previous release matches the bahaviour I've come to expect - which is that if you want a newline you add "\n". Without that you don't get one.

So this shows one line:

 10 PRINT "HELLO"
 20 PRINT " "
 30 PRINT "WORLD"

frodo ~ $ gobasic t.bas 
HELLO WORLDfrodo ~ $ 

But this does the right thing:

 10 PRINT "HELLO, WORLD\n"

That might surprise some old sample programs, but I do recall having to use things like this, back in the past, so perhaps not:

 10 PRINT "Steve", CHR$ 13

In conclusion I think I'll remove the mandatory newline, and I'll drop the DUMP primitive which was basically me testing that the builtin-stuff worked, when I moved as much as I could out of the core.

I trued to build your UI the other night, but it failed due to Glx issues. I'll try again later, because it seems like it will be a fun project when you get a little further along - I see the todo regarding I/O for example :)

(I did consider a renumber-utility, but never got round to it. Rewriting the lines is trivial, but having to parse the code to update the GOTO/GOSUB instructions would be fiddly and annoying :) )

andydotxyz commented 5 years ago

I appreciate that this project has a history and that needs to be considered - but I didn't realise that you were expecting a PRINT declaration to always be inline. Check out the JSBeeb example at https://bbc.godbolt.org/ which does what I had understood.

screen shot 2019-03-05 at 21 56 59

If you need to keep the old behaviour then let's document it clearly and I can make a wrapper PRINT on my end that handles newline and ";" in the other way.

--

On the "TODO" side of things - should I add support for error output as well before release? i.e. an Environment.StdError() bufio.Writer?

skx commented 5 years ago

I can definitely see that embedded users will have different preferences, and needs.

So I'd be happy with either :

Whichever you prefer is fine.

As for the error-handling, yes that is probably a good idea. At the moment there are a few places in the core where we output debugging information via fmt.Printf - but the only one that seems like it will be user-visible is:

  fmt.Printf("WARN: Line %s is duplicated - GOTO/GOSUB behaviour is undefined\n", line)

The rest of the error-handling comes from catching the err parameter from the lexer/parser/interpreter. So I guess you could handle that specially if you were embedded.

In short: I don't think there's more to fix than one obvious case, but adding a stderr wrapper/default and using it is probably sane and reasonable.

andydotxyz commented 5 years ago

It's interesting to note that the INPUT command does require a newline, rather than just terminating on nil or other termination event - should we try to make them consistent? Sorry that I missed the example files - I was just running the tests and they all passed. Should we be running some examples within a test and validating the output?

I can make a PR that addresses the PRINT newline, and adds the error out stream - more for future use or external commands than the builtins I suspect. I'll have that in a day or s

skx commented 5 years ago

It's interesting to note that the INPUT command does require a newline, rather than just terminating on nil or other termination event - should we try to make them consistent?

I'm not sure I understand your point here? I thought you were talking about the prompt but the the mention of termination event / nil confuses me ..

Sorry that I missed the example files - I was just running the tests and they all passed. Should we be running some examples within a test and validating the output?

That's not a bad idea. I'm in the github actions beta and can add it there. Though of course a naive go test ./... won't run them.

I can make a PR that addresses the PRINT newline, and adds the error out stream - more for future use > or external commands than the builtins I suspect. I'll have that in a day or s[o]

Wonderful. Thank-you :)

andydotxyz commented 5 years ago

I'm not sure I understand your point here? I thought you were talking about the prompt but the the mention of termination event / nil confuses me ..

Apologies I was meaning this: https://github.com/skx/gobasic/blob/master/eval/eval.go#L1707 - it requires a newline on the input, so the INPUT vs PRINT feels mismatched?

skx commented 5 years ago

Now I understand, thanks.

In my view PRINT's newline handling and the requirement of a newline for INPUT aren't the same thing. After all input reads a line of input in all BASIC implementations I'm familiar with, so it must read until the user presses return. If you wanted to read just a single character you'd usually use use INKEY/INKEY$ instead.

(I haven't implemented INKEY because I have no real-time/interactive utility/program in the repository. I setup the HTTP-server as a compromise between writing a UI, as you have, and having it all be file-based. I figured writing a decent interactive system/emulator of something would be more work than I wanted, because I just wanted to experiment with a-BASIC rather than dedicate a lot of time to sorting out the UI niggles and extra compatibility with a real-system. Right now I pretend I'm ZX Spectrum, but there are enough quirks and omissions that's only broadly true.)

andydotxyz commented 5 years ago

Sorry I'm not trying to prolong this discussion - but your response made me think more about this... "reading until the user presses return" assumes an input/output stream in the same way that I was assuming newline would be used for the output :). In the examples you have "\n" at the end of every line so it's clearly a common requirement.

I think this is an artefact of the way that the tests needed a stin for running tests and were not able to verify the equivalent output. Should that be proliferated - or should it be change to be consistent one way or the other (either with, or without, trailing \n)...

Just a thought

andydotxyz commented 5 years ago

I hope that #101 addresses these issues - apologies for the delay. I did not touch the INPUT as that seems to be less related to PRINT newlines than I had thought.