refreshViewLinesIfNeeded re-wrapped every line of the buffer whenever
the view was tainted. That's cheap for short content, but scrolling a
long diff calls it constantly: adjustDownwardScrollAmount queries
ViewLinesHeight on every scroll event, and each newly-read line taints
the view, so every notch re-wrapped the entire buffer. Wrapping measures
each cell's width (uniseg) and allocates per line, so once you'd scrolled
far enough down the diff, scrolling turned sluggish - the cost grew with
how much had been read. (A CPU profile of scrolling deep in a long diff
put 77% of the time in lineWrap, reached almost entirely via
ViewLinesHeight rather than draw.)
Cache each line's wrapped result on the lineType, keyed by the width it
was wrapped at, and only re-wrap lines that have actually changed since
the last refresh. A firstDirtyLine index, updated in the same three
places that set `tainted` (write, clearViewLines' callers, SetHighlight),
marks the lowest line that might have changed; lines below it with a
matching cached width reuse their cached wrapping. The cache lives on the
line, so it's freed with the line when the view's content is replaced
(e.g. selecting a different commit) - it doesn't accumulate across a
session.
The wrapping cost per scroll now scales with the number of lines just
read, not with the total size of the buffer, so scrolling stays smooth
no matter how far down you are.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The next commits move refresh workers to read UI-thread-owned state (the
model, contexts, selection) on the UI thread rather than off it. Two
primitives support that:
- OnUIThreadAndWait runs a function on the main event loop and blocks the
caller until it has run, so a worker can read that state without racing.
OnUIThreadAndWaitBackground is the same for background routines, whose
work must not count towards the program being busy.
- IsUIThread reports whether the caller is on the main event loop, for a
debug-only assertion that a refresh was issued from the thread it claims.
It records the main loop's goroutine id in MainLoop and compares via
goid, so it's promoted from an indirect to a direct dependency.
goid is used only by that debug assertion, never to drive production
control flow.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The repo-switch busy query must not count view-buffer content rendering:
those tasks paint a view rather than drive a git operation, so leaving
one running across a switch is harmless (the switch's own refresh
re-renders). More importantly, they fire on nearly every focus/selection
change — including the context activation that runs right before a
menu/prompt confirmation handler (e.g. confirming worktree creation).
A synchronous busy check in such a handler would otherwise see that
render and make the very switch the handler is about to request refuse
itself.
Route ViewBufferManager's tasks through a new gocui NewBackgroundTask so
they're tracked for idle detection but excluded from the busy query. The
task "background" flag now covers two kinds of non-blocking work: the
background routines (and their refreshes) tagged earlier, and view
rendering.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Repo-switch safety needs to answer, synchronously on the UI thread,
"is any foreground work in flight right now?" so it can refuse a switch
that would run against a repo about to be swapped out. gocui already
tracks a task per OnWorker/Update for the test idle-listener; extend
that.
Tasks gain a background flag: background tasks (the ongoing routines
like auto-fetch, and the refreshes they trigger) don't count towards
busy, because their model writes are already guarded against a
concurrent switch by the repo generation. Add OnWorkerBackground,
UpdateBackground and UpdateContentOnlyBackground (plus the gui-layer
OnUIThreadBackground / OnUIThreadContentOnlyBackground / OnWorkerBackground
on IGuiCommon) so the few background call sites can opt in without
touching the hundreds of foreground callers.
TaskManager.hasBusyForegroundTaskExcept answers the query; Gui.Busy()
wraps it, excluding the event currently being processed (recorded as
currentTask) so a handler asking the question doesn't count itself.
Nothing gates on Busy() yet; this is the mechanism only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ConPTY compresses runs of default-colored spaces into ECH + CUF
(\x1b[NX\x1b[NC) instead of emitting them literally. ECH is still a
no-op for us — our buffer is built sequentially and has nothing to
erase — but CUF has to materialize as N visible space cells so the
gap actually appears, otherwise content the child wrote with leading
indentation slides left against the preceding cell.
The view's cursorForward branch reuses the same machinery as tab
expansion: substitute the trigger byte for a space and let the
repeatCount path emit the cells under the parser-tracked SGR. The
existing notifyCellsWritten plumbing then advances screenCol over
the gap, keeping subsequent CUP targets aligned.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConPTY compresses runs of default-colored spaces into ECH + CUF
(\x1b[NX\x1b[NC) rather than emitting them literally. Both currently
fall through the parser's swallow path, so the gap they describe
collapses entirely and content that the child wrote with leading
indentation ends up slid left against the previous cell.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConPTY presents its child's output as a screen buffer and uses CUP /
CUD / CNL / VPA to skip over blank rows rather than emitting LFs. The
previous behaviour swallowed all of those and the visible content
collapsed together. Now the escape parser tracks the screen-relative
cursor row, and any CSI that moves the cursor past the current row
emits a cursorDown instruction that the view turns into the matching
number of empty lines.
Column tracking is deliberately omitted: doing it correctly would mean
duplicating the view's grapheme-cluster width math in the parser, and
ConPTY in practice positions to column 1 after a CR-equivalent, which
the existing wx-reset path already handles. ConPTY-internal scrolling
needs no special handling either: it only emits cursor-positioning
escapes within the first, un-scrolled screenful — once its screen
scrolls it switches to plain linefeeds, which the view advances on
directly regardless of the tracked cursor.
Backward cursor moves are silently dropped — the view's buffer is
append-style and can't undo earlier writes. The exception is cursor-home
(CUP to row 1): ConPTY emits it at the start of every screen, so rather
than drop it we re-anchor the row tracking to the current write position.
Without that, a view not rewound in lockstep with ConPTY's screen (the
command log, which streams pty output without a rewind) accumulates
drift, and every later absolute CUP becomes a dropped backward move that
collapses the rows ConPTY positioned with.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConPTY presents its child's stdout as a screen buffer and uses CUP
(`\x1b[<row>;<col>H`) to skip over blank rows rather than emitting LFs
for them. Our escape interpreter swallows CUP via the catch-all
"valid CSI final byte we don't implement" branch, so the blank rows
the child put between non-blank ones disappear and the surrounding
lines collapse together — which is what makes the delta-rendered diff
in the screenshot look like its blank lines and section breaks were
removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update spawned a goroutine per call that then sent on the user-events
channel, so multiple Update calls from the same goroutine could be
reordered by the scheduler — the doc comment even admitted "the order in
which the user events will be handled is not guaranteed." That
non-determinism is a latent source of flaky rendering: code that queues a
model update and then a render in source order could see them run in the
opposite order.
Send on the channel directly instead, so same-goroutine calls arrive in
source order. The send is non-blocking and panics on a full channel
rather than blocking (a blocked send from the UI goroutine would deadlock
against itself) or silently reordering; the buffer is sized generously so
this is unreachable in normal use. UpdateAsync is now identical to Update
and unused, so it's removed along with the shared updateAsyncAux helper.
After the previous commit, the escape interpreter still had five paths
that returned an error from parseOne, which view.go handles by rendering
whatever bytes it had accumulated as literal cells. Each of these is a
case where silently consuming the sequence is strictly better than
leaking garbage.
- ';' as the first CSI byte: '\x1b[;5H' is a valid sequence (row
defaults to 1) but we errored on the leading ';'.
- Intermediate bytes in CSI ('\x1b[0 q' = DECSCUSR): the sequence ends
in a final byte we don't implement, so consume and drop.
- Malformed SGR params (empty slot like '\x1b[1;;m'): if outputCSI
fails mid-parse, reset state instead of re-emitting the sequence.
- OSC 8 that isn't actually OSC 8 ('\x1b]8x...'): treat as an OSC we
don't understand and skip to its terminator rather than error-
resetting mid-sequence, which used to leave the rest of the OSC body
to be printed as text.
- The sanity-check overflow paths (too many params, param too long)
now switch to a 'discard until final byte' state rather than
returning the accumulated bytes.
A new stateCSIDiscard centralizes the 'consume bytes until the CSI
final' behavior used by both the intermediate-byte and overflow paths.
errCSITooLong and errOSCParseError are gone with their only callers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A text-mode escape interpreter can't do anything meaningful with cursor
positioning, DEC private modes, or terminal resets — but it must still
consume them, not print them as literal text. Before this change, any
sequence outside SGR / EL / OSC-8 errored out of parseOne, and view.go
rendered the unparsed bytes as visible cells. On Windows this would show
up as junk at the start of main-panel output once we add PTY support
using ConPTY, because ConPTY's session-init stream is full of such
sequences.
Three additions to the state machine:
- stateEscape: a single byte in 0x30–0x7E after ESC (e.g. ESC c = RIS)
is a complete Fs/Fp sequence per ECMA-48; consume and reset.
- stateCSI: accept the DEC private-mode prefix bytes (<, =, >, ?), and
accept a CSI final byte (0x40–0x7E) immediately after [ as the end of
a zero-param sequence.
- stateParams: accept any CSI final byte we don't implement as the end
of the sequence rather than a parse error.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The escape interpreter errors on anything outside the handful of
sequences it understands (SGR, EL, OSC 8 hyperlinks), and view.go then
renders the unparsed bytes as text cells.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lazygit reloads changed config files when its terminal window regains
focus, but the test harness had no way to simulate that focus event, so
the live config-reload path was untestable. Add a focus event to the
replayed-events queue and expose it through the GuiDriver as FocusIn.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tools like delta paint each diff line's background with '\x1b[K' so
the color reaches the right edge. Up to now the '\x1b[K' handler
appended (InnerWidth - cx) explicit padding cells with the fill bg
so rendering picked up the color. That worked for short lines but
silently degraded once content exceeded InnerWidth: the repeat
count went non-positive, no cells were added, and after wrapping
the partial tail segment was left without any cells carrying the
fill color, so draw() fell back to the view's default bg.
Record the fill colors on the source line as optional
trailingFillAttributes. In the '\x1b[K' handler set them (and drop
the padding-cell loop — the metadata covers both the wrap and the
non-wrap cases). In draw(), once per source line, pick the trailing
cell's fg/bg from the metadata if present and otherwise from the
view defaults; then the inner-loop fills past-content cells with
that.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The sentinel was appended to every \n-terminated line solely so that
draw()'s prevFgColor tracking would reset to default for the trailing
area; without it, an AttrReverse-styled last cell would carry its
rendered bg past the end of the line.
The same prevFgColor mechanism propagated AttrReverse past content on
*unterminated* lines too — which doesn't match real terminal behavior
(try `print '\x1b[7m\x1b[31mfoo'` in a shell: the reverse stops at
the last character) and isn't relied on by anything in lazygit, since
all our writers terminate lines with \n.
Drop the sentinel cell, drop prevFgColor, and just have draw() paint
trailing cells with the view's default fg/bg. The
TestUnterminatedReverseLineExtendsToEdge regression test inverts to
document the new (terminal-matching) behavior, renamed accordingly.
TestWriteString expectations also drop the trailing "" that came
from the sentinel.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cells of a source line will soon need to carry metadata about how
the line was terminated (newline vs filled to edge via \x1b[K). Move
to a struct so there's somewhere to put it; this commit only renames
[][]cell to []line{cells: ...} with no behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The four ReplaceAll(str, "\x00", "") calls (and the equivalent
rune-by-rune skip in linesToString) are leftover from when cell.chr
was a rune and \x00 was used as an internal sentinel. With chr now
being a string and no code path writing \x00, the filtering never
strips anything.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tools like delta emit each diff line with the bg color set, then
\x1b[K to fill the rest of the row with that bg color. When the
content fits within the view's inner width, gocui's \x1b[K handling
appends explicit padding cells and rendering works. When the content
exceeds the inner width, \x1b[K adds no cells (negative repeat
count), the line is wrapped into multiple segments, and the partial
tail segment's trailing cells fall back to the view default bg
instead of continuing the fill color.
Add a test that drives draw() against a tcell mock terminal and
asserts the current (buggy) trailing background.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The next few commits restructure how the view's draw() decides the
fg/bg of cells past the end of a line's content. Pin down three
existing behaviors first so the restructuring stays a refactor:
- '\n' should reset attributes for the trailing area so a reversed
final cell doesn't bleed into empty space.
- An unterminated line with AttrReverse on its last cell should
propagate that to the right edge (otherwise the rendered bg
abruptly stops at the last character).
- '\x1b[K' on a line that fits within InnerWidth should fill the
remaining cells with the current bg color.
Introduce a small WithSimulationScreen helper that swaps in a tcell
mock terminal so tests can call view.draw() and inspect rendered
cells via Screen.Get().
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Until now every keybinding config field was a plain string. That meant a user
couldn't ask for two keys to invoke a command — the config silently accepted
only one form.
Convert every string-typed field across all 13 KeybindingXxxConfig structs to
Keybinding so the union type extends to every command. Defaults wrap their
single-key value in Keybinding{...} so the generated Config.md still renders one
scalar key per binding.
The alt fields keep their separate Binding registrations for now: this commit
does not yet introduce the merge mechanism that folds them into the main field —
that comes in a follow-up. Consumers previously calling opts.GetKeys on a string
field now call opts.GetKeys on the Keybinding, or take .String() / Keys[0] where
a single value is needed.
Adds a Keybinding.String helper for rendering, schema-generator work that
inlines the Keybinding union into each consuming property, and a unit test
covering the user-facing scalar/sequence YAML forms for quit.
This prevents views from drawing over higher z-order views.
Currently this is not an issue in practice, because we use
ForceFlushViewsContentOnly only for the bottom line status spinner, and there
are never views on top of it. However, later in the branch we will use the
mechanism to redraw the inline spinners in panels (e.g. the "Pushing..." status
next to a branch name), and there could be a popup on top of it.
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
Modifiers were moved into Key in 22169e22f, but the separate Modifier field
on types.Binding and gocui.keybinding was left behind. The keypress matcher
already compares modifiers via Key.Equals, so the old field is never read on
the dispatch path; it just got passed through SetKeybinding and stored.
Drop it from gocui.keybinding, types.Binding, and the SetKeybinding /
DeleteKeybinding signatures, and remove every now-redundant Modifier:
gocui.ModNone struct field. Mouse bindings keep their own Modifier (on
ViewMouseBinding) since that path still consults it.
I noticed that it compares key.keyName and key.str separately instead of calling
key.Equals, but instead of deciding whether that's a problem, just delete the
function when nobody needs it.
These have alternate keys (<ctrl+h> for backspace, and <ctrl+f> for
move-cursor-right). Both of these broke with commit 22169e22ffc46c5; the first
because it was accidentally omitted, the second because of a stupid copy/paste
mistake.
This changes not only how we store modifiers (inside of Key instead of passing
it separately), but also how we parse keybinding strings: it supports all
combinations of modifiers now (if the terminal supports it, that is).
This bundles the keyName and a rune, so that we don't have to pass these around
separately everywhere. This should make it easier to swap out the rune for a
string when we upgrade to tcell v3.
I copied all files except dot files (.github and .gitignore), the _examples
folder, and go.mod/go.sum.
At some point we may want to copy the files back to the gocui repo when other
clients (e.g. lazydocker) want to use the newer versions of them.
