Move SetContentLineCount into OverwriteLinesAndClearEverythingElse. Calling it
separately beforehand is not concurrency safe; we need both to happen
when the view's writeMutex is locked.
It is possible to scroll the selection out of view using the mouse wheel; after
doing this, it would sometimes scroll into view by itself again, for example
when a background fetch occurred. In the files panel this would even happen
every 10s with every regular files refresh.
Fix this by adding a scrollIntoView parameter to HandleFocus, which is false by
default, and is only set to true from controllers that change the selection.
This fixes a crash in an interactive rebase when there's a merge command in the
rebase-todo file that doesn't have a comment. I don't know under what
circumstances this can happen; git itself doesn't produce these, but it is
theoretically possible for the user to do this manually by doing `git rebase
--edit-todo`, or third-party tools could do it too.
We had one user report a crash because of this, so it seems worth fixing it.
Adaptions are for this gocui commit:
Cleanup: remove Is* error functions
- Use errors.Is instead of quality comparisons. This is better because it
matches wrapped errors as well, which we will need later in this branch.
- Inline the errors.Is calls at the call sites. This is idiomatic go, we don't
need helper functions for this.
See https://go.dev/blog/go1.13-errors for more about this.
Since they started releasing .0 versions with 1.21.0, the go version
need to be a full release version specifier and not a go language
version. They relented and added support for defaulting to a .0 release
if none is specified in later 1.23.x releases, but for users with local
1.21 or 1.22 toolchains the lack of a full release specifier breaks
them. And going forwards this is more technically correct anyway.
See https://github.com/golang/go/issues/62278#issuecomment-1693620853
This updates gocui to include https://github.com/jesseduffield/gocui/pull/68 and
https://github.com/jesseduffield/gocui/pull/69, which changes views to not have
an extra blank line at the end when content ending in a newline character is
written to them. This makes text views more consistent with list views, which
don't have a blank line after the last list entry either.
Original commit message of the gocui change:
This fixes View.Size, Width and Height to be the correct (outer) size of a view
including its frame, and InnerSize/InnerWidth/InnerHeight to be the usable
client area exluding the frame. Previously, Size was actually the InnerSize (and
a lot of client code used it as such, so these need to be changed to InnerSize).
InnerSize, on the other hand, was *one* less than Size (not two, as you would
have expected), and in many cases this was made up for at call sites by adding 1
(e.g. in calcRealScrollbarStartEnd, parseInput, and many other places in the
lazygit code).
There are still some weird things left that I didn't address here:
- a view's lower-right coordinates (x1/y1) are one less than you would expect.
For example, a view with a 2x2 client area like this:
╭──╮
│ab│
│cd│
╰──╯
in the top-left corner of the screen (x0 and y0 both zero) has x1/xy at 3, not
4 as would be more natural.
- a view without a frame has its coordinates extended by 1 on all sides; to
illustrate, the same 2x2 view as before but without a frame, sitting in the
top-left corder of the screen, has coordinates x0=-1, y0=-1, x1=2, y1=2. This
is highly confusing and unexpected.
I left these as they are because they would be even more of a breaking change,
and also because they don't have quite as much of an impact on general app code.
An inactive selection is one where the view is part of the context stack, but
not the active view. For example, the files view when you enter the staging
panel, or any view when you open a panel.
Sometimes it takes a while to get PRs accepted upstream, and this blocks our
progress. Since I'm pretty much the only one making changes there anyway, it
makes sense to point to my fork directly.
In Gui.onWorker we only make the minimum possible change to get things to
compile after the API-breaking change of the gocui update; we'll make this
cleaner later in this branch.
The previous scrollbars were too chunky and encroached too much on a view's content.
The new ones are slim and right-aligned so they encroach into dead space between views
which is much better
This adds range select ability in two ways:
1) Sticky: like what we already have with the staging view i.e. press v then use arrow keys
2) Non-sticky: where you just use shift+up/down to expand the range
The state machine works like this:
(no range, press 'v') -> sticky range
(no range, press arrow) -> no range
(no range, press shift+arrow) -> nonsticky range
(sticky range, press 'v') -> no range
(sticky range, press arrow) -> sticky range
(sticky range, press shift+arrow) -> nonsticky range
(nonsticky range, press 'v') -> no range
(nonsticky range, press arrow) -> no range
(nonsticky range, press shift+arrow) -> nonsticky range
Previously there was no way to render a view's search status without also moving the cursor
to the current search match. This caused issues where we wanted to display the status
after leaving the view and coming back, or when beginning a new search from within the
view.
This commit separates the two use cases so we only move the cursor when we're actually
selecting the next search match
... and import stefanhaller's tcell fork for real rather than just replacing it
This solves the problem that people trying to
"go install github.com/jesseduffield/lazygit@latest" would get the error
go: github.com/jesseduffield/lazygit@latest (in github.com/jesseduffield/lazygit@v0.40.0):
The go.mod file for the module providing named packages contains one or
more replace directives. It must not contain directives that would cause
it to be interpreted differently than if it were the main module.
Now that we refresh upon focus, we can scrap this file watching code.
Stefan says few git UIs use file watching, and I understand why: the
reason this code was problematic in the first place is that watching
files is expensive and if you have too many open file handles that
can cause problems.
Importantly: this code that's being removed was _already_ dead.
From the go 1.19 release notes:
Command and LookPath no longer allow results from a PATH search to be found relative to the current directory. This removes a common source of security problems but may also break existing programs that depend on using, say, exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe) in the current directory. See the os/exec package documentation for information about how best to update such programs.
Afero is a package that lets you mock out a filesystem with an in-memory filesystem.
It allows us to easily create the files required for a given test without worrying about
a cleanup step or different tests tripping on eachother when run in parallel.
Later on I'll standardise on using afero over the vanilla os package
This fixes the issue in accordion mode where the current line wasn't in the viewport upon focus.
It doesn't perfectly fix it: the current line always appears at the top of the view. But it's good enough
to cut a new release. The proper fix is to only focus the line after the view has had its height adjusted.
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).
A better refactor would be to allow matchers to assert against either a string or a slice of cells, so that I could have
the same ergonomics that I have elsewhere, but this is a start.
