Fix #2394 - Expose terminal cursor

[Garek33]

Fixes #2394

• new Suffixes for terminal mostly as described in the Issue
• I decided to implement put as a suffix, too. But I don't personally have a strong opinion on that and would be willing to make it a builtin or implement terminal:println and terminal:printat for consistency.
• New testscript: terminalcursor.ks to test the new suffixes
• Possibly unexpected behaviour:
  • Setting cursorcol or cursorrow on the interpreter leads to the prompt moving around (but, what else would you want to achieve with that?)
  • If a script leaves cursorcol on a non-zero value, the Program ended-Message is offset by that

[Dunbaratu]

Hmm. This might explain a few other weird behaviours I've seen in the past.

[Dunbaratu]

The problem with making a built-in for "print without linebreak" is that we already used "print" for "print with linebreak" so something like "print" and "println" would break every old script.

I think at this point we are kind of stuck with being unable to do a nice built-in for "print no line feed".

We could add a "putln" suffix to terminal, alongside the "put" suffix you made.

Then we just change the documentation to say that the built-in print command is really just an alias for terminal:putln("string").

[Dunbaratu]

The original decision, made before I got to the project, to make "PRINT AT" NOT move the cursor to the new location, is one of those things I'd have done differently if we had the luxury of starting from scratch. If it did move the cursor, that would have made things make more sense when trying to work with old terminal escape codes.

[Garek33]

The problem with making a built-in for "print without linebreak" is that we already used "print" for "print with linebreak" so something like "print" and "println" would break every old script.

That's exactly why I called it put.

I think at this point we are kind of stuck with being unable to do a nice built-in for "print no line feed".

We could add a "putln" suffix to terminal, alongside the "put" suffix you made.

Then we just change the documentation to say that the built-in print command is really just an alias for terminal:putln("string").

Sounds good, I'll add that to the PR... propably tomorrow. Fits with the general "everything's a structure" that seems to be going on in kOS these days (not that I don't like that, quite the opposite).

The original decision, made before I got to the project, to make "PRINT AT" NOT move the cursor to the new location, is one of those things I'd have done differently if we had the luxury of starting from scratch.

While I'm at it, I would change the documentation of print at to emphasise it not moving the cursor and link to the new terminal suffixes.

[Dunbaratu]

This is all looking good so far. I have been slow to review because of a cold this week making my brain all stuffy and miserable feeling.

I think I'd prefer it if there was nothing print or print at does that a terminal suffix can't also do, so that the "grammar" commands (commands that require keyword support in the parser, like PRINT AT), are really truly just aliases to proper function calls. To that ends, I think the print at behaviour needs to be duplicated in the terminal suffixes too. Right now, using the terminal suffixes , you can't print "at" a spot without the side effect of also moving the cursor there. Maybe we need a TERMINAL:PUTAT(string, col, row) suffix that does the same as print at - namely that it prints at a spot, but leaves the cursor where it was.

[Garek33]

This is all looking good so far. I have been slow to review because of a cold this week making my brain all stuffy and miserable feeling.

Thank you. I hope you're feeling better now!

I think I'd prefer it if there was nothing print or print at does that a terminal suffix can't also do, so that the "grammar" commands (commands that require keyword support in the parser, like PRINT AT), are really truly just aliases to proper function calls. To that ends, I think the print at behaviour needs to be duplicated in the terminal suffixes too. Right now, using the terminal suffixes , you can't print "at" a spot without the side effect of also moving the cursor there. Maybe we need a TERMINAL:PUTAT(string, col, row) suffix that does the same as print at - namely that it prints at a spot, but leaves the cursor where it was.

Added that. Although it's just a new suffix to terminal. The parser code for print is unchanged, even though we could refactor that to call the terminal suffixes instead of the globals and remove those.

Also added a terminal:movecursor(col,row) so one can move the cursor in a single call as opposed to two sets.

[Dunbaratu]

We can leave it as two implementations for now. The refactor to merge them can come later.

[lucaelin]

TERMINAL:PUT("Please enter a number: "). set c to TERMINAL:INPUT:GETCHAR(). TERMINAL:PUT(c).

Running this code in the interactive terminal causes only the (visible) cursor to advance, but no chars to show. When you now type new text into the terminal, that text appears at the beginning of the line instead of where the cursor ended up being.

[Garek33]

So what actually happens is that the interactive prompt uses its own linebuffer, that then replaces the line the cursor is currently in, hiding any output there. We don't want that. This also means not clearing the line the cursor is placed in - if a kerboscript dev puts the cursor in the middle of existing output, they are responsible for that, we enable that by not hiding output unexpectedly.

I currently think a comparatively elegant solution would be to just have TextEditor write to the main buffer of ScreenBuffer, like everything else. Obviously need to check if that behaves well etc. Hopefully I get to that tomorrow, but could take a few days.

[Garek33]

Something also to check, does put work correctly if the argument string contains new lines. Looking at ScreenBuffer again, I would actually expect it not to.

(this is mostly a reminder for myself, although anybody testing this might want to also look at that)

EDIT: just tested this, currently newlines as part of the argument string to put get ignored. With putln or print they do work as expected. Also, if interactive input wrapped around, and then gets reduced so it does no longer wrap around, the last character to be removed from the later line is not.

[Garek33]

{print
"something".}

If one inputs the above repeatedly in the interactive terminal, with varying amounts of empty lines between the two lines of actual code, previous output is sometimes overwritten with the echoed input, after the input is confirmed. I.e. the echoed input looses empty lines and sometimes moves up in the buffer. So it seems the Interpreter/TextEditor/ScreenBuffer-Stack is already broken for some edge cases, this "just" adds some more of those. Not that I won't try to fix it.

[Garek33]

Continuing a discussion from the Discord here for better documentation.

So @Dunbaratu and I agreed that scrolling the view should no longer move the cursor and instead the view should jump to the cursor when print output happens.

Following from that, I think that neither print at nor script-side cursor manipulation should use the current view as a reference frame anymore. I see two possibilities to implement that consistently:

(A) On print at or if anything happens with the cursor, scroll to the end of output, then use that view as the reference frame. This would be consistent with print at operating within terminal:height and always being visible to the user. However, if something adds another line to the output, this still shifts existing output underneath the reference frame. Meaning, one still can't reliably overwrite specific existing output while there's possibly additional output happening.

(B) Change print at and the cursor to use absolute coordinates, i.e. relative to the start of output. This would enable scripts to reliably overwrite previous output, regardless of further output at the end of the buffer. This would 'break' scripts that expect print at output using an absolute position without a preceding clearscreen to be visible to the user. However, since clearscreen wipes all output, using absolute positions after a clearscreen still works like previously. In this case I would not auto-scroll to the position of a print at, because multiple print ats from different parts of a script could be more than terminal:height away from each other and I don't want them to fight over the scroll position.

I'm personally in favour of variant B. Opinions?

[Garek33]

This is not ready yet, with the last commit the following issues still remain:

• terminal output suffixes doesn't autocast to string
• in some scenarios (I'm not exaclty sure when) scrolling down after a clearscreen makes the cursor vanish
• scrolling in a telnet client produces some minor artifacts
• creating a new throwaway SubBuffer every frame for merging purposes is probably not a good idea performance-wise

So there will be some additional, hopefully smaller, commits to this.

[Dunbaratu]

@Garek33 I sort of forgot about this PR since the last thing you said on it was "this is not ready yet". You mentioned that a few more commits to address remaining problems were coming, and I saw a few more commits after that on Feb 10, but no follow-up message saying, "I now think this is ready" so I wasn't looking at it yet.

As I now go through older PR's in the history, I see this and wonder, ,"Okay, so were those commits the end of it? Was it ready after that and I just didn't know it was ready?"

[Garek33]

@Garek33 I sort of forgot about this PR since the last thing you said on it was "this is not ready yet". You mentioned that a few more commits to address remaining problems were coming, and I saw a few more commits after that on Feb 10, but no follow-up message saying, "I now think this is ready" so I wasn't looking at it yet.

As I now go through older PR's in the history, I see this and wonder, ,"Okay, so were those commits the end of it? Was it ready after that and I just didn't know it was ready?"

Sorry about dropping off without notice like that, but... life happened. (And apparently missing github notifications, sorry again). As far as I remember, I left this unfinished.
I'm actually still in the middle of a move at the moment, so nothing happening right now, although I intend to get back into KSP/KOS once everythings settled in. I would estimate an actual update on this happening somewhere around September.

[Garek33]

Hi, sorry about leaving this unfinished for so long, but life happened and quite frankly, I wasn't playing KSP anymore. I updated the pull request:

• resolved conflicts with the base branch
• minor code improvements
• added additional tests/examples

After that and some manual testing I think this is ready.

Some things I noticed, but I think are out of scope for this PR:

• In an interactive shell (both ingame and telnet) tab advances the visible cursor, but adds no character.
• Reducing the width wraps lines instead of truncating, which can look weird. Especially because increasing it doesn't unwrap, but extend.
• If a script uses this to create its own shell, it needs to render it's own cursor. I'm fine with this being the case, but maybe in the future, we could allow a script to enable/disable the builtin cursor rendering?