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.
The previous commit already fixed the user-visible lag, but there's still a
problem with multiple background git processes consuming resources calculating
diffs that we are never going to show. Improve this by terminating those
processes (by sending them a TERM signal).
Unfortunately this is only possible on Linux and Mac, so Windows users will have
to live with the higher CPU usage. The recommended workaround is to not use
"diff.algorithm = histogram".
One reason why git diff can be very slow is when "diff.algorithm = histogram" is
being used. In this case, showing a very long single-file diff can take seconds
to load, and you'll see the "loading..." message in the main view until we got
the first lines of the diff to show. There's nothing really we can do about this
delay; however, when switching to another, shorter file (or commit) while the
"loading..." message is still showing, this switch should be instantaneous. And
it was before 0.54.0, but we broke this in 0.54.0 with 8d7740a5ac (#4782); now
users have to wait for the slow git diff command to output more text before the
switch occurs.
To fix this, don't block waiting for the process to terminate if we just stopped
it.
Now that we close a task's stdout pipe when we are done with it, it should
terminate by itself at that point, so there's no longer a need to kill it. This
way, called processes get a chance to terminate gracefully rather than being
killed with SIGKILL; in particular, this allows git to clean up its index.lock
file if it created one.
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
And only while the task is running.
This avoids accumulating lots of blocked goroutines when scrolling a view down
more than 1024 times (the capacity of the readLines channel).
This may lead to unrelated processes being killed on Windows (https://github.com/jesseduffield/lazygit/issues/3008). Imagine:
1. lazygit is started and runs git diff in process X which completes immediately and exits.
2. lazygit is left in the background for several hours by which process X pid is reused by an unrelated process.
3. lazygit is focused back on and runs another git diff. It first runs this stop logic which will kill process X and its children.
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.
This changes GetRepoPaths() to pull information from `git rev-parse`
instead of effectively reimplementing git's logic for pathfinding. This
change fixes issues with bare repos, esp. versioned homedir use cases,
by aligning lazygit's path handling to what git itself does.
This change also enables lazygit to run from arbitrary subdirectories of
a repository, including correct handling of symlinks, including "deep"
symlinks into a repo, worktree, a repo's submodules, etc.
Integration tests are now resilient against unintended side effects from
the host's environment variables. Of necessity, $PATH and $TERM are the
only env vars allowed through now.
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.
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).
We had some test flakiness involving the index.lock file which is fixed by this commit.
We shouldn't be accessing newTaskID without the mutex, although I'm surprised that this
actually fixes the issue. Surely we don't have tasks (which typically render to the main
view) which use index.lock?
We refresh the view after reading just enough to fill it, so that we see the
initial content as quickly as possible, but then we continue reading enough
lines so that we can tell how long the scrollbar needs to be, and then we
refresh again. This can result in slight flicker of the scrollbar when it is
first drawn with a bigger size and then jumps to a smaller size; however, that's
a good tradeoff for a solution that provides both good speed and accuracy.
