When scrolling a lazy-loaded view (a diff in the main view, the command
log, etc.), we top up the view's line buffer by reading more lines from
the still-running task. This was driven by asking the task to read a
fixed number of *additional* lines on every scroll event, which had two
problems:
- It was decoupled from the scroll position. Scrolling down, back up,
and down again re-read lines that had already been read, so the buffer
crept towards the end of the input regardless of where the user
actually scrolled.
- A single wheel notch only bought a single notch worth of runway, so
fast scrolling constantly outran the reader and had to wait for the
next read (and re-render) on every notch.
Make ReadLines take an absolute target total instead of a delta: the
task tracks how many lines it has read and only reads the shortfall, so
requests are idempotent. Callers now ask to fill the viewport at the
current scroll position plus a few screenfuls of read-ahead, which gives
scrolling enough runway to stay smooth.
The four call sites all wanted the same "fill this view" computation, so
consolidate them into a single ReadLinesToFillView helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GuiRepoState.mergeOrRebaseStartedInLazygit and StartupStage are plain
fields, but they're written and read from worker goroutines: the former
from both the files refresh and the merge/rebase result path (which runs on
a worker for the async callers), the latter from the reflog/branches load as
it transitions the startup stage. Those are data races.
Make both atomic, like Branch.BehindBaseBranch. They're leaf flags, not
mutexes guarding model or view state, so an atomic is the natural fit and
keeps the merge/rebase result path out of this change.
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>
refreshStateFiles now does its git work on the worker and enqueues a
single OnUIThread closure that writes Model.Submodules, Model.Files, and
the FileTreeViewModel state together, instead of writing them directly
from the worker goroutine. refreshStateSubmoduleConfigs becomes a pure
getter (returns the configs; no model write) so the result can be
threaded into that same bounce.
The STAGING handler wraps RefreshStagingPanel in OnUIThread after
fileWg.Wait() so it sees the post-bounce file model rather than the stale
pre-refresh one — without this it would race the files bounce queued just
above it.
Bouncing the write opens a hazard the old synchronous write didn't have:
if the user switches repos while this refresh is in flight, the queued
closure would fire after resetState has replaced the model with a fresh
one for the new repo, silently overwriting it with the previous repo's
files. Guard against this with a repo generation: resetState bumps a
counter on every switch, refreshStateFiles captures it before its git
work, and onUIThreadUnlessRepoChanged drops the bounce if the generation
has moved on. This one helper is the general mechanism the remaining
scopes' bounces will use too; the same guard covers the rebase-continue
prompt, which reads Model.Files right after.
A generation counter, not a comparison of the *Model pointer: switching
away from and back to a repo reuses that repo's cached state (the same
Model pointer), which a pointer comparison would wrongly accept even
though the in-flight data is stale.
PromptToContinueRebase's Then callback (previous commit) now gets an
explanatory comment, since this is the commit that makes it necessary.
The explicit locking around these writes (RefreshingFilesMutex in
refreshFilesAndSubmodules, FileTreeViewModel.RWMutex around the write in
refreshStateFiles) is left in place for now even though it's becoming
redundant, to keep this commit focused on the bounce itself; it's removed
next.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GetIsRefreshingFiles() is never called anywhere in the codebase, so the
flag serves no purpose. Remove it from Gui, StateAccessor, and
IStateAccessor, and drop the two SetIsRefreshingFiles calls in
refreshFilesAndSubmodules that maintained it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Move the pty master behind a small interface (Read/Write/Close/Resize),
and push the actual startup into a platform-specific StartPty function
in pkg/commands/oscommands. The Unix implementation still uses
creack/pty; the Windows implementation is a stub that returns
ErrPtyUnsupported, at which point newPtyTask falls back to a plain cmd
task — matching the existing Windows behavior.
The primitive lives in oscommands rather than pkg/gui because the
cmd_obj_runner pty handler (also in oscommands) is going to consume it
too, and tasks → oscommands is the existing dependency direction.
Same observable behavior on every platform; this just carves out a seam
for a real ConPTY implementation on Windows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When conflicts of an in-progress rebase/merge/cherry-pick/revert are
resolved, lazygit pops up a prompt offering to continue it. This is
helpful when you started the operation in lazygit and resolved the
conflicts in your editor. But it's confusing when the operation was
started outside lazygit — e.g. by a coding agent in another terminal
that resolves the conflicts but hasn't continued yet because it's still
running tests or fixing the build. lazygit would then prompt unbidden.
Track whether the in-progress operation was started from within lazygit,
and only show the prompt in that case. We record this right after running
a merge/rebase step (in CheckMergeOrRebaseWithRefreshOptions, the
subprocess branch of genericMergeCommand, and the custom-command
conflict path), and clear it whenever a refresh observes that no
operation is in progress — which also handles an operation that was
finished or aborted externally.
The conflict-resolution tests start their operation by running git
directly (not through lazygit's UI), so they call the new test helper
Common.PretendMergeOrRebaseStartedInLazygit to have lazygit treat the
operation as its own and still get the prompt.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When the config file changes and lazygit regains focus it reloads the
config, but the side panel window assignments, default views, tab
strips, and z-order were only ever set up on repo entry, so a changed
sidePanels wouldn't take effect until restart. Re-apply it from the
reload path: reassign windows and default views and restore each panel's
default tab.
The focused panel needs care: resetting it to its default tab would
leave the focused tab hidden behind that default tab, so the panel looks
unfocused even though its tab is selected. Re-focus the current context
so its tab stays shown and highlighted; only when the new config hides
the focused panel entirely do we move focus to the default side panel.
Tab strips are already refreshed via configureViewProperties.
Replace the hard-coded side panel order, tab groupings, and window
assignments with values resolved from the gui.sidePanels config. The panel
order (SideWindows and the layout boxes), the tab strips (viewTabMap), the
per-context window names, each window's default view, and the jump-label
groups all now come from the config rather than from five separate
hard-coded lists.
A panel's window name is the name of its first tab, and panels not listed
in the config get their own window name so their views stay hidden instead
of overlapping a visible panel. Three small lookups translate config names
into views, tab titles, and contexts; a test keeps them in sync with the
set of valid names. The lookups are split this way (rather than one
resolver) because configureViewProperties runs before the context tree
exists, so the title/view lookups must not depend on it.
The config is applied to a repo's contexts via applySidePanelConfig on
every repo entry, including the cached-repo path: a repo's per-repo config
can differ from the previously visited one's, so each repo's contexts must
be (re)assigned from its own config rather than kept from when they were
first built.
With the default config this reproduces today's layout exactly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a 2-second background poll that calls Status.RefsSnapshot and
compares against the snapshot stored at the end of the last refs-
touching refresh. On a diff, trigger a full refresh — same scope as the
focus-in handler, because once we know something changed externally
we can't be sure what (an agent might have created a worktree or
stashed something alongside the commit we detected).
Refresh runs in SYNC mode because goEvery already serializes iterations
via <-done: a slow refresh delays the next tick naturally instead of
letting work stack. The post-refresh hook from the previous commit
updates the snapshot, so in-app commands don't cause the next poll to
spuriously re-fire.
Disabled in the integration test config, like autoRefresh and autoFetch,
because demo replays make repo changes throughout the run; at 2-second
cadence the resulting full refreshes compete with the demo's own
choreography and push some demos past their 40-second timeout.
Also list the two new config keys in checkForChangedConfigsThatDontAutoReload
so a config edit warns the user that lazygit needs a restart.
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 didn't cause a bug so far because switching repos always happens from
within an OnWaitingStatus, so the spinner would take care of calling layout and
draw. However, later in this branch we are going to optimize the spinner so that
it no longer calls layout, at which point this would break, so make sure we
rerender at the point where it's needed.
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.
Not a terribly important feature for users, probably, but it is useful for
developers when testing a new feature with or without nerd fonts; and it does go
against or policy of all configs being hot-reloadable.
Note that switching from nerd fonts version 2 to 3 is *not* supported without
restarting, but I find this less important.
When switching to a different repo, and then back to the original one, searching
would no longer work. The reason is that our contexts set callbacks on their
views; when switching to a different repo we instantiate a new set of contexts,
so they will overwrite the views' callbacks with their new ones, but when
switching back to the original repo, we reuse the old contexts because they are
still in memory, but they won't set their callbacks again since they only do
this on construction.
To fix this, replace the view-local callbacks with a global one on the gui that
takes the view as an argument, so that the callback can look up the associated
context dynamically.
This fixes two problems:
- if the previously focused view (behind the panel) was a list view, it would
look like the click would select a different row, because gocui would still
set the view's cursor position, which is used to draw the highlighted row
- it was still possible to click on tab headers, and this would dismiss the
panel
Tweak regexp such that a trailing ":" is not included in the file path
extracted from a lazygit-edit URL.
Previously, when matching a URL containing the ":" separator but no line
number, such as `lazygit-edit:///path/to/file.ext:`, the trailing
separator would be included in the matched file path, so lazygit would
open the non-existent file `/path/to/file.ext:`. Notably, such urls are
created when using delta with the hyperlink feature, as suggested in
https://github.com/jesseduffield/lazygit/blob/master/docs/Custom_Pagers.md#delta,
and clicking a file path rather than a line number.
This is an object that is owned by Gui, is accessible through GuiCommon.State(),
and also passed down to GitCommand, where it is mostly needed. Right now it
simply wraps access to the Git.Paging config, which isn't very exciting, but
we'll extend it in the next commit to handle a slice of pagers (and maintain the
currently selected pager index), and doing this refactoring up front allows us
to make that change without having to touch clients.
So far, confirmations and prompts were handled by the same view, context, and
controller, with a bunch of conditional code based on whether the view is
editable. This was more or less ok so far, since it does save a little bit of
code duplication; however, now we need separate views, because we don't have
dynamic keybindings, but we want to map "confirm" to different keys in
confirmations (the "universal.confirm" user config) and prompts (hard-coded to
enter, because it doesn't make sense to customize it there).
It also allows us to get rid of the conditional code, which is a nice benefit;
and the code duplication is actually not *that* bad.
Refresh is one of those functions that shouldn't require error handling (similar
to triggering a redraw of the UI, see
https://github.com/jesseduffield/lazygit/issues/3887).
As far as I see, the only reason why Refresh can currently return an error is
that the Then function returns one. The actual refresh errors, e.g. from the git
calls that are made to fetch data, are already logged and swallowed. Most of the
Then functions do only UI stuff such as selecting a list item, and always return
nil; there's only one that can return an error (updating the rebase todo file in
LocalCommitsController.startInteractiveRebaseWithEdit); it's not a critical
error if this fails, it is only used for setting rebase todo items to "edit"
when you start an interactive rebase by pressing 'e' on a range selection of
commits. We simply log this error instead of returning it.
I took the set of enabled checks from revive's recommended configuration [1],
and removed some that I didn't like. There might be other useful checks in
revive that we might want to enable, but this is a nice improvement already.
The bulk of the changes here are removing unnecessary else statements after
returns, but there are a few others too.
[1] https://github.com/mgechev/revive?tab=readme-ov-file#recommended-configuration
Instead, pass the entire Mutexes struct by pointer to controllers.
This is better because Mutexes now has a zero value and doesn't need to be
initialized.
When switching between repos, each repo might have a different focused panel; in
this case, the previously focused panel would show the "inactive" highlight. By
default this is only bold text, so it's barely noticeable, but it becomes more
pronounced when setting e.g.
gui:
theme:
inactiveViewSelectedLineBgColor:
- "#666666"
I noticed this especially when entering or leaving submodules; for example,
enter a submodule by pressing enter in the Files panel, then switch to the
Commits panel in the submodule, then press Esc to go back to the parent repo.
This would put the focus back into the Files panel, but keep the inactive
highlight in the Commits panel.
This in itself is not an improvement, because hashes are unique (they are shared
between real commits and rebase todos, but there are so few of those that it
doesn't matter). However, it becomes an improvement once we also store parent
hashes in the same pool; but the real motivation for this change is to also
reuse the hash pointers in Pipe objects later in the branch. This will be a big
win because in a merge-heavy git repo there are many more Pipe instances than
commits.
Apparently this was an attempt at working around go's lack of default arguments,
but it's very unidiomatic and a bit confusing. Make it a normal parameter
instead, so all clients have to pass it explicitly.
This allows changing git config values while lazygit is running (e.g. in a
different terminal tab, or even in lazygit's ":" shell prompt), and have them
take effect immediately, while still getting some benefit from caching them
while lazygit is in the foreground.