This test-only function wraps testWriteString with semantic prompt
marking. This replaces the manual, row-based semantic_prompt field
manipulation we were doing in all of our prompt-related test setups.
This function's heuristics are a little complex because it wraps
testWriteString as a "black box"; we don't benefit from that function's
own line-based logic to know which rows need to be updated with the
semantic prompt flag. We need to infer them externally instead.
I considered adding an options argument to testWriteString that would
allow passing e.g. a semantic_prompt prompt. Given that it's called from
200+ places, that would involve a lot of unrelated changes, but it
remains an "option" (ha!) if there's value there for other cases.
I also have plans that move us from row-based to cell-based semantic
tracking, where the current semantic type is tracked by the cursor. In
that implementation, testWriteString can update the written cells
directly, and testWriteSemanticString just helps manage the cursor's
state. Introducing testWriteSemanticString here and now therefore helps
bridge us to that world while maintaining test consistency.
These tests write specific lines into a 10-column-wide test screen. The
"prompt3$ input3\n" writes exceed that column limit, and some of their
characters wrap onto the following line.
These tests' current assertions aren't sensitive to that overflow, but
I spotted the problem while doing some related work, and I thought it
worth making these corrections to avoid any future surprises.
Cleans up the logic, checks for out of bounds using rows instead of
sel.contains because that excludes cases where a rectangle selection
doesn't include the leftmost column.
Also adds test for clipping behavior of rectangular selections.
Fixes an issue where rectangle selections would appear visually wrong if
their start or end were out of the viewport area, because when cloning
them the restored pins were defaulting to the start and end of the row
instead of the appropriate column.
This commit changes a LOT of areas of the code to use decl literals
instead of redundantly referring to the type.
These changes were mostly driven by some regex searches and then manual
adjustment on a case-by-case basis.
I almost certainly missed quite a few places where decl literals could
be used, but this is a good first step in converting things, and other
instances can be addressed when they're discovered.
I tested GLFW+Metal and building the framework on macOS and tested a GTK
build on Linux, so I'm 99% sure I didn't introduce any syntax errors or
other problems with this. (fingers crossed)
Fixes#7066
This fixes an issue where under certain conditions (expanded below), we
would not clear the correct row, leading to the screen having duplicate
data.
This was triggered by a page state of the following:
```
+----------+ = PAGE 0
... : :
4305 |1ABCD00000|
4306 |2EFGH00000|
:^ : = PIN 0
+-------------+ ACTIVE
4307 |3IJKL00000| | 0
+----------+ :
+----------+ : = PAGE 1
0 | | | 1
1 | | | 2
+----------+ :
+-------------+
```
Namely, the cursor had to NOT be on the last row of the first page,
but somewhere on the first page. Then, when an `index` (LF) operation
was performed the result would look like this:
```
+----------+ = PAGE 0
... : :
4305 |1ABCD00000|
4306 |2EFGH00000|
+-------------+ ACTIVE
4307 |3IJKL00000| | 0
:^ : : = PIN 0
+----------+ :
+----------+ : = PAGE 1
0 |3IJKL00000| | 1
1 | | | 2
+----------+ :
+-------------+
```
The `3IJKL` line was duplicated. What was happening here is that we
performed the index operation correctly but failed to clear the cursor
line as expected.
This is because we were always clearing the first row in the page
instead of the row of the cursor.
Test added.
clearCells() always asserts its page's integrity after finishing its
work (via a `defer`). We don't need to re-assert the page's integrity
immediately thereafter.
Fixes#5718
When a terminal is resized with text reflow (i.e. soft-wrapped text), the cursor
is generally reflowed with it.
For example, imagine a terminal window 5-columns wide and you type the
following without pressing enter. The cursor is on the X.
```
OOOOO
OOX
```
If you resize the window now to 8 or more columns, this happens, as expected:
```
OOOOOOOX
```
As expected, the cursor remains on the "X". This behaves like any other text
input...
Terminals also provide an escape sequence to
[save the cursor (ESC 7 aka DECSC)](https://ghostty.org/docs/vt/esc/decsc).
This includes, amongst other things, the cursor position. The cursor can be
restored with [DECRC](https://ghostty.org/docs/vt/esc/decrc).
The behavior of the position of the _saved cursor_ in the context of text
reflow is unspecified and varies wildly between terminals Ghostty does this
right now (as do many other terminals):
```
OOOOOOOO
X
```
This commit changes the behavior so that we reflow the saved cursor.
Fully reset the kitty image storage instead of using the delete handler,
previously this caused a memory corruption / likely segfault due to use
of undefined, since the delete handler tries to clear the tracked pins
for placements, which it gets from the terminal, for which `undefined`
was passed in before.
Previously assumed adjusted capacities would always fit the cursor style
and hyperlink, this is not necessarily the case and led to memory
corruption in scenarios with large hyperlinks.
Extracted from #3110
Initial fix is relatively basic, and catching it with a test only
required a little bit of extra scrutiny of the cursor state after one of
the tests that we already had.
However, the fix revealed faulty dirty tracking logic throughout the
`cursorScrollAbove` function (and therefore bad results that were being
tested for when they should not have been). I've ended up clarifying
things, fixing the asserted dirty states in all the `cursorScrollAbove`
tests, and then finally implementing another very trivial fix that
catches the mistake.
Fixing the dirty tracking is really just an exercise in correctness
though, since when the scroll happens it inherently invalidates the
viewport, and therefore will trigger a full rebuild in the renderer...
unless, I guess, another operation is performed that cancels things out
and results in the viewport pin being in the same place as the previous
render, but that seems an exceptionally difficult scenario to make
happen on purpose much less accidentally.
This PR is almost entirely changes to comments and tests, there are only
2 lines of real code it changes, the one added to the start of
`cursorScrollAbove` and the one modified at the start of
`cursorScrollAboveRotate`. I believe these changes are entirely safe. (I
wonder if they might have a bad effect on our `vtebench` scrolling
performance though...)
Accounts for improved behavior due to prior memory corruption fix for
`cursorScrollAboveRotate` and reveals a new problem in a different
`cursorScrollAbove` sub-function.
Unless it's guaranteed that the new pin is in the same page as the old
one, `cursor.page_pin` should only be updated through `cursorChangePin`,
not directly.
We call `cursorChangePin` which may invalidate the provided pin if it
needs to adjust the page capacity, and as such we should consider the
pin we pass in to it invalid afterwards, and access it through cursor
instead.
Fixes#2841
We were incorrectly applying the start/end x offset for the first/last
row of every single page. If a selection spanned multiple pages this
would trim data incorrectly.
Unit test updated to cover this case.
Our semantic prompts are row-based, so the last prompt marker set on a
row "wins". In the case of at least our bash shell integration, this
means that consecutive prompt lines will all be marked as .input (OSC
133;B -- end-of-prompt, start of input).
Previously, clearPrompt() identified the current prompt's "area" by
searching upward from the current row until it encounters a .prompt
marker or some command output. In the bash case, .prompt is never the
dominant ("last") marker, so clearPrompt() would aggressively clear all
immediately preceding consecutive prompts.
With this change, we'll stop searching upwards when we encounter some
command output, a .prompt marker, _or another .input marker_. That last
case prevents clearPrompt() from unintentionally clearing earlier prompt
lines.
There may be improvements we can make to the way that our bash shell
integration emits semantic prompt markers, but I think this logic is
generally sound for all cases, and it specifically improves the current
bash prompt-clearing experience.
In multiple tests we create 1 or more pages by growing them 1 row at a
time, which results in an integrity check of the page for each row grown
which is just... horrible. By simply pausing integrity checks while
growing these pages (since growing them is not the point of the test) we
MASSIVELY speed up all of these tests.
Also reduced grapheme bytes during testing and made the Terminal "glitch
text" test actually assert what it intends to achieve, rather than just
blindly assuming 100 copies of the text will be the right amount -- this
lets us stop a lot earlier, making it take practically no time.
This is more correct: a pagelist is a linked list of nodes, not pages.
The nodes themselves contain pages but we were previously calling the
nodes "pages" which was confusing, especially as I plan some future
changes to the way pages are stored.
Fixes#2500
Based on #2508
This separates out the concept of a "hyperlink" from a "hyperlink page
entry." The difference is that the former has real Zig slices into
things like strings and the latter has offsets into terminal page
memory.
From this separation, the Page structure now has an `insertHyperlink`
function that takes a hyperlink and converts it to a page entry.
This does a couple things: (1) it moves page memory management out of
Screen and into Page which is historically more appropriate and (2) it
let's us more easily test hyperlinks from the Page unit tests.
Finally, this PR makes some error sets explicit.
When a screen is resized, the pages are generally reallocated. This
causes the cursor hyperlink state to be lost and ultimately the
hyperlink ref count is off by one.
The unit test in this commit showcases the issue very clearly. And you
can see we do this logic already for styles. We never copied it over for
hyperlinks.
