We've been sometimes using lo and sometimes using my slices package, and we need to pick one
for consistency. Lo is more extensive and better maintained so we're going with that.
My slices package was a superset of go's own slices package so in some places I've just used
the official one (the methods were just wrappers anyway).
I've also moved the remaining methods into the utils package.
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
We want to be using forward slashes everywhere internally, so if we get a path from windows
we should immediately convert it to use forward slashes.
I'm leaving out the recent repos list because that would require a migration
There are quite a few paths you might want to get e.g. the repo's path, the worktree's path,
the repo's git dir path, the worktree's git dir path. I want these all obtained once and
then used when needed rather than having to have IO whenever we need them. This is not so
much about reducing time spent on IO as it is about not having to care about errors every time
we want a path.
This fixes pkg/integration/tests/worktree/rebase.go which was failing on old git versions due to a difference in
order of branches that don't have recency values
Older versions of git don't support the -z flag in `git worktree list`.
So we're using newlines.
Also, we're not raising an error upon error because that triggers another refresh,
which gets us into an infinite loop
This can be useful if you want to find the commit that fixed a bug (you'd use
"broken/fixed" instead of "good/bad" in this case), or if you want to find the
commit that brought a big performance improvement (use "slow/fast"). It's pretty
mind-bending to have to use "good/bad" in these cases, and swap their meanings
in your head.
Thankfully, lazygit already had support for using custom terms during the bisect
(for the case that a bisect was started on the command-line, I suppose), so all
that's needed is adding a way to specify them in lazygit.
Now that we run code concurrently in our loaders, we need to handle that in our tests.
We could enforce a deterministic ordering by mocking waitgroup or something like that,
but I think it's fine to let our tests handle some randomness given that prod itself
will have that randomness.
I've removed the patch test file because it was clunky, not providing much value, and
it would have been hard to refactor to the new pattern
We're:
* using concurrency with wait groups
* avoiding regex
* processing lines of input as they come rather than storing everything in one string
* avoiding an inner loop by creating a mapping of remote names to branches
The speedup is most noticeable on first load, when we haven't yet fetched out main branches.
I saw a speedup from 105ms to 60ms. On subsequent loads the gain is more modest;
54ms to 40ms
Previously we used a single-line prompt for a tag annotation. Now we're using the commit message
prompt.
I've had to update other uses of that prompt to allow the summary and description labels to
be passed in
As discussed in https://github.com/jesseduffield/lazygit/pull/2599, it
makes more sense to have the user specify whether they want verbose
commits from their own git config, rather than lazygit config.
This means that we can remove all the code (including test coverage)
associated with the custom verbose flag, and lazygit will just inherit
the .gitconfig settings automatically.
Save has been deprecated for a while, push is the recommended way to save a
stash. Push has been available since 2.13, so we can use it without problems.
The code in getHydratedRebasingCommits relied on the assumption that the
git-rebase-todo file contains full SHAs. This has only been true from 2.25.2 on,
before that it would contain abbreviated SHAs. Fix this by storing fullCommits
in a slice instead of a map, and using a linear search.
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).
The first line of the diff pane would show branch heads (e.g.
commit dd9100ccc8b69a8b14b21a84e34854b5acfb871a (mybranch, origin/mybranch)
only when a pager is used. The reason is that the default of the --decorate
option to git show is "auto", which means to show the decoration only when
output goes to a tty. Lazygit uses a pty only when a pager is used, so the
decoration wouldn't show when no pager is used.
Since the branch head annotation is useful and we always want to see it, force
it by explicitly passing --decorate.
This solves three problems:
1. When the local main branch is behind its upstream, the merged status of
commits of a feature branch sitting on origin/main was not correct. This can
easily happen when you rebase a branch onto origin/main instead of main, and
don't bother keeping local main up to date.
2. It works when you don't have the main branch locally at all. This could
happen when you check out a colleague's feature branch that goes off of
"develop", but you don't have "develop" locally yourself because you normally
only work on "main".
3. It also works when you work on a main branch itself, e.g. by committing to it
directly, or by merging a branch locally. These local commits on a main
branch would previously be shown in green instead of red; this broke with
910a61dc46.
When stopping in a rebase because of a conflict, it is nice to see the commit
that git is trying to apply. Create a fake todo entry labelled "conflict" for
this, and show the "<-- YOU ARE HERE ---" string for that one (in red) instead
of for the real current head.
Missed a spot a couple PR's ago. We had an integration test which caught this but which was skipped due
to index.lock file issues. The test was also broken for other reasons due to it not having been running
for a while, so I've fixed that up too.
By constructing an arg vector manually, we no longer need to quote arguments
Mandate that args must be passed when building a command
Now you need to provide an args array when building a command.
There are a handful of places where we need to deal with a string,
such as with user-defined custom commands, and for those we now require
that at the callsite they use str.ToArgv to do that. I don't want
to provide a method out of the box for it because I want to discourage its
use.
For some reason we were invoking a command through a shell when amending a
commit, and I don't believe we needed to do that as there was nothing user-
supplied about the command. So I've switched to using a regular command out-
side the shell there
The only exception is when moving a custom patch for an entire commit to an
earlier commit; in this case the source commit becomes empty, but we want to
keep it, mainly for consistency with moving the patch to a later commit, which
behaves the same.
In all other cases where we rebase, it's confusing when empty commits are kept;
the most common example is rebasing a branch onto master, where master already
contains some of the commits of our branch. In this case we simply want to drop
these.
It defaults to {"master", "main"}, but can be set to whatever branch names
are used as base branches, e.g. {"master", "devel", "v1.0-hotfixes"}. It is
used for color-coding the shas in the commit list, i.e. to decide whether
commits are green or yellow.
Previously, when rebasing a branch onto a newer master, all commits from the
previous fork point up to its head were marked red (unpushed), including the
commits that are on master already. While this is technically correct from the
perspective of the current branch's upstream, it's not what most people expect,
intuitively; they want to see where the current branch starts, relative to
master. So all commits of master should be green, and then the commits of the
current branch in red.
When we use the one panel for the entire commit message, its tricky to have a keybinding both for adding a newline and submitting.
By having two panels: one for the summary line and one for the description, we allow for 'enter' to submit the message when done from the summary panel,
and 'enter' to add a newline when done from the description panel. Alt-enter, for those who can use that key combo, also works for submitting the message
from the description panel. For those who can't use that key combo, and don't want to remap the keybinding, they can hit tab to go back to the summary panel
and then 'enter' to submit the message.
We have some awkwardness in that both contexts (i.e. panels) need to appear and disappear in tandem and we don't have a great way of handling that concept,
so we just push both contexts one after the other, and likewise remove both contexts when we escape.
This fixes two problems with the "amend commit with staged changes" command:
1. Amending to a fixup commit didn't work (this would create a commmit with the
title "fixup! fixup! original title" and keep that at the top of the branch)
2. Unrelated fixup commits would be squashed too.
The added integration test verifies that both of these problems are fixed.
Instead of passing a bunch of different options in
PrepareInteractiveRebaseCommandOpts, where it was unclear how they interact if
several are set, have only a single field "instruction" which can be set to one
of various different instructions.
The functionality of replacing the entire todo file with our own is no longer
available; it is only possible to prepend todos to the existing file.
Also, instead of using different env vars for the various rebase operations that
we want to tell the daemon to do, use a single one that contains a json-encoded
struct with all available instructions. This makes the protocol much clearer,
and makes it easier to extend in the future.
At the moment it doesn't make a big difference, because the vast majority of
callers create a list of todos themselves to completely replace what git came up
with. We're changing this in the following commits though, and then it's helpful
to preserve merges.
It used to work on the assumption that rebasing commits in lazygit's model
correspond one-to-one to lines in the git-rebase-todo file, which isn't
necessarily true (e.g. when users use "git rebase --edit-todo" at the custom
command prompt and add a "break" between lines).
This is useful when working with stacked branches, because you can now move
"pick" entries across an update-ref command and you can tell exactly which
branch the commit will end up in.
It's also useful to spot situations where the --update-refs option didn't work
as desired. For example, if you duplicate a branch and want to rebase only one
of the branches but not the other (maybe for testing); if you have
rebase.updateRefs=true in your git config, then rebasing one branch will move
the other branch along. To solve this we'll have to introduce a way to delete
the update-ref entry (maybe by hitting backspace?); this is out of scope for
this PR, so for now users will have to type "git rebase --edit-todo" into the
custom command prompt to sort this out.
We will also have to prevent users from trying to turn update-ref commands into
other commands like "pick" or "drop"; we'll do this later in this branch.
So far the algorithm worked on the assumption that the output of the "git show"
command corresponds one-to-one to the lines of the rebase-todo file. This
assumption doesn't hold once we start to include todo lines that don't have a
sha (like update-ref), or when the todo file contains multiple entries for the
same sha. This should never happen normally, but it can if users manually edit
the todo file and duplicate a line.
The main reason for doing this (besides the reasons given for Status in the
previous commit) is that it allows us to easily convert from TodoCommand to
Action and back. This will be needed later in the branch. Fortunately,
TodoCommand is one-based, so this allows us to add an ActionNone constant with
the value 0.
This is unrelated to the changes in this PR, but since we are doing the same
thing for the commit.Action field in the next commit, it makes sense to do it
for Status too for consistency. Modelling this as an enum feels more natural
than modelling it as a string, since there's a finite set of possible values.
And it saves a little bit of memory (not very much, since none of the strings
were heap-allocated, but still).
