Dropping a hunk with `d` re-establishes the focused main view's selection
on the next surviving change, which feels great. But other operations that
rewrite the commit under the focused main view — moving a custom patch out
into the index, undoing right after a discard or a patch move — don't run
through the focused-main-view action handlers, so nothing was preserving
the selection. The stale gocui selection was left painted over the new
content, often as a large, now-meaningless range.
Rather than teach every such command to capture and restore the selection
(move-patch, undo, redo, and any future one), the focused main view now
preserves it itself, by its change-line ordinal, as the diff re-renders —
the command-agnostic counterpart of revealSelectionAfterPrimaryAction. The
diff side panels call it from their render-to-main before triggering the
render, so the restore rides the re-render.
It stands down unless the focused main view is current and shows a
selection, no precise restore is already pending (escape / post-stage
reveal / context-size place the selection more precisely), and the diff
command is actually changing. That last gate matters: a plain background
refresh re-renders the same commit's diff unchanged, and there the
selection — range and all — must be left alone; only a command change
(e.g. a rebase rewriting the commit's hash) means the content moved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire the remaining diff panels — local commits, sub-commits, commit files,
stash, and reflog — into the focused main view's raw-diff fallback, the same way
the files panel already is: each computes DiffMainViewShouldRenderRaw and renders
its diff raw (no pager) when focused under a pager whose output we can't resolve,
so its selection stays toggleable into a custom patch (or, for reflog, navigable
and copyable).
The commit-diff panels build their diff through the diff helper, which can't
reach the staging helper, so the panel computes renderRaw and passes it in.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Until now space toggled lines into a custom patch only from the commit files
main view (the per-file diff). Register the same toggle handler on the commits,
sub-commits and stash panels, so a patch can be built straight from the
whole-commit (multi-file) diff their main view shows, without first diving into
the commit files panel.
The handler lives on SwitchToDiffFilesController, which is already bound to
exactly those three panels and already knows how to derive the patch target
(from/to/reverse/canRebase) for the selected ref — pulled out of enter() into a
shared canRebase helper and the reused FromAndToForDiff. The post-toggle refresh
is the cheap one: these panels have no per-file patch indicator to update, so we
just re-render their own main + secondary views rather than reloading the whole
commit list on every keystroke.
Sub-commits and stash didn't render the secondary patch view at all; give them
the same secondaryPatchPanelUpdateOpts the commits panel uses, and recompute the
inclusion gutter as their diff (re-)renders, so the cumulative patch and the
gutter track the toggle.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With sync vs async now derived from the calling thread, the Mode field
and its SYNC/ASYNC constants no longer carry any information: Refresh is
always async, RefreshFromWorker always sync. Drop the field, the type,
and the Mode argument at every call site, and reduce the debug log's
mode name to a plain sync/async derived from calledFromWorker.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
In other views that show lists of commits (reflog and stash) it doesn't make
sense to show a range diff of selected entries because they don't form a linear
sequence, so we keep the previous behavior of showing the diff for the free end
of the selection range in those view.
The same applies to the commits view if the selection range includes rebasing
todos; these can have an arbitrary order, and a range diff doesn't make sense
for those.
This lets us get rid of a few more calls to Error(), and it simplifies things
for clients of OnWorker: they can simply return an error from their callback
like we do everywhere else.
Something dumb that we're currently doing is expecting list items
to define an ID method which returns a string. We use that when copying
items to clipboard with ctrl+o and when getting a ref name for diffing.
This commit gets us a little deeper into that hole by explicitly requiring
list items to implement that method so that we can easily use the new
helper functions in list_controller_trait.go.
In future we need to just remove the whole ID thing entirely but I'm too
lazy to do that right now.
We want to show an error when the user tries to invoke an action that expects only
a single item to be selected.
We're using the GetDisabledReason field to enforce this (as well as DisabledReason
on menu items).
I've created a ListControllerTrait to store some shared convenience functions for this.
The global counter approach is easy to understand but it's brittle and depends on implicit behaviour that is not very discoverable.
With a global counter, if any goroutine accidentally decrements the counter twice, we'll think lazygit is idle when it's actually busy.
Likewise if a goroutine accidentally increments the counter twice we'll think lazygit is busy when it's actually idle.
With the new approach we have a map of tasks where each task can either be busy or not. We create a new task and add it to the map
when we spawn a worker goroutine (among other things) and we remove it once the task is done.
The task can also be paused and continued for situations where we switch back and forth between running a program and asking for user
input.
In order for this to work with `git push` (and other commands that require credentials) we need to obtain the task from gocui when
we create the worker goroutine, and then pass it along to the commands package to pause/continue the task as required. This is
MUCH more discoverable than the old approach which just decremented and incremented the global counter from within the commands package,
but it's at the cost of expanding some function signatures (arguably a good thing).
Likewise, whenever you want to call WithWaitingStatus or WithLoaderPanel the callback will now have access to the task for pausing/
continuing. We only need to actually make use of this functionality in a couple of places so it's a high price to pay, but I don't
know if I want to introduce a WithWaitingStatusTask and WithLoaderPanelTask function (open to suggestions).
This begins a big refactor of moving more code out of the Gui struct into contexts, controllers, and helpers. We also move some code into structs in the
gui package purely for the sake of better encapsulation
