Add TerminalBuffer & TerminalCursor, parse \n \r \b, and more escape codes

[ian-mcdowell]

This change adds another layer between the Parser and TerminalTextView called the TerminalBuffer (referred to as the "buffer"). The buffer contains an NSTextStorage (NSMutableAttributedString subclass) that the TerminalTextView reads from.

The buffer is responsible for adding new text that comes in to the proper position in the text storage. It uses the TerminalCursor (referred to as the "cursor") to keep track of where it needs to insert new text.

Parsing

The buffer is now the Parser's delegate, and receives events from the Parser, some of which are:

• Newline found (\n)
• Carriage return found (\r)
• Attributed text found

Depending on what the Parser sends, the buffer will update the cursor's position and/or modify the text storage.

Cursor

The cursor (different than the UITextView's cursor), keeps track of a position within the text storage. It's only valid for a single text storage. It has a readable x,y coordinate, as well as an offset (current distance from the beginning of the storage).

The cursor contains mutating methods that adjust its state, and is not able to be modified except through those methods. Every time the x, y, or offset change, it makes sure to keep the other values in sync.

Example

In this example, I'll demonstrate what happens when a carriage return is received while parsing.

At the beginning, the storage contains a 2-line string, and the cursor is right after the last character of the first word:

text storage = "iPad: \nhello world"
cursor = { x = 5, y = 1, offset = 12 }

Now, when a carriage return is received, the parserDidReceiveCarriageReturn(_:) method is called by the Parser. After this is handled, here is the state:

text storage = "iPad: \nhello world"
cursor = { x = 0, y = 1, offset = 7 }

Now, if more text comes in from the parser - "bonjour", here is the end state:

text storage = "iPad: \nbonjourorld"
cursor = { x = 7, y = 1, offset = 14 }

[ian-mcdowell]

I decided to disable the swiftlint error in this method instead of splitting it up into smaller methods. This method basically models a state machine, and doesn't do much other than change states and call delegate methods. It would, in my opinion, reduce readability a lot if I were to move the inner switches into methods and call those.

@louisdh thoughts?

[louisdh]

That's fine.

[ian-mcdowell]

Now, switching background colors while terminal is open will not keep remnants of the background color visible where text was printed.

[ian-mcdowell]

Tests were added for both new classes. The TerminalBufferTests is pretty limited at the moment, but the TerminalCursorTests has good code coverage.

[ian-mcdowell]

One regression I didn't address here:

The OutputSanitizer (replace /private/var/mobile/Containers/.../Documents with ~) is incompatible with the new way that text flows into the buffer, as it happens character by character now, much like other terminals handle text.

Does anyone have feedback on what should be done? Is that replacement necessary? We could sanitize after every character is printed, but that seems too expensive.

[louisdh]

The OutputSanitizer is quite necessary for a nice user experience in my opinion. On top of that, it might be required for App Store approval. I'm speculating here, but Apple may not like an app displaying paths containing "/private/var/mobile/...".

[louisdh]

Great work btw, making OpenTerm behave more like established terminals is a nice improvement. Also good to see some tests.

[ian-mcdowell]

Thanks! Assuming that the sanitizer won't ever need to replace multiple lines at once, we could avoid displaying each character as it comes in. Whenever we receive \r, \n, etc, or the input ends, run the sanitizer then display the full line .

[ian-mcdowell]

This is ready to merge now.

• Fixed a bug with outputting emoji, where the cursor would get offset when used with NSRange
• Fixed a history saving bug
• Re-added the OutputSanitizer

[louisdh]

Two unit tests are failing in the latest commit of this branch:
• testBasicText
• testTextWithNewLine

[ian-mcdowell]

Sorry about that, tests were broken with OutputSanitizer change. They are back up now 🙂

[louisdh]

Is this good to merge?

[ian-mcdowell]

We will need additional validation before it ships, as it may break some commands. Lets keep it in this branch for a while, until I have more confidence in its correctness.

[louisdh]

Now that OpenTerm 2.0 is done, it would be great to get this PR up to date. Would it be possible to fix all these conflicts?

[ian-mcdowell]

I’ll take a look ASAP!