Unlike moving a patch to the index, applying or reverting a patch didn't
auto-stash, which means that applying a patch when there's a modified (but
unstaged) file in the working tree would error out with the message "error:
file1: does not match index", regardless of whether those modifications conflict
with the patch or not.
To fix this, we *could* add auto-stashing like we do for the "move patch to
index" command. However, in this case we rather simply stage the affected files
(after asking for confirmation). This has a few advantages:
- it only changes the staging state of those files that are contained in the
patch (whereas auto-stashing always changes all files to unstaged)
- it doesn't unnecessarily show a confirmation if none of the modified files are
affected by the patch
- if the patch conflicts with the modified files, the conflicts were "backwards"
("ours" was the patch, "theirs" the modified file); it is more logical if "ours"
is the current state of the file, and "theirs" is the patch.
It's a little unfortunate that the behavior isn't exactly the same as for "move
patch to index", but for that one we do need the auto-stash because of the
rebase that runs behind the scenes.
The tests show that this currently fails with the confusing error message "does
not match index", regardless of whether the patch conflicts with the
modifications or not. We'll improve this in the next commit.
I don't bother adding tests for reverting a patch, as the code is basically the
same as for apply.
This is functionality that works already, we only add the test for more complete
test coverage. However, there's a detail problem, and the test demonstrates
this: we keep the stash even if there was no conflict. We'll fix this next.
The function would return "head/branchname" when there was either a tag or a
remote with the same name.
While fixing this we slightly change the semantic of the function (and of
determineCheckedOutBranchName, which calls it): for a detached head it now
returns an empty string rather than the commit hash. I actually think this is
better.
The icon will appear when there's a tag with the same name as the current branch
(that's what we're testing here), or even when there's a remote with the same
name. I'm not adding a test for this latter case, but this was actually how I
discovered the issue.
Pretty basic fix, didn't seem to have any complications.
I only added the refs/ prefix to the FullRefName() method
to align with other similar methods, and to make this change
not impact any user facing modals.
Fixes: https://github.com/jesseduffield/lazygit/issues/4634
Also adds a test demonstrating that the stash show behavior is now fixed
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.
This now allows for leaving the status panel and returning back to the
same log command. Previously any return to the status panel would result
in the next command in the list being shown. Now, you need to press `a`,
with a log command being rendered, to rotate to the next
allBranchesLogCmd.
BeginInteractiveRebaseForCommit is used for all the patch commands, and for
rewording. It works by setting the commit we want to stop at to 'edit'; this
doesn't work for merge commits. This wasn't a problem for the patch commands so
far, because you typically don't use custom patches with merge commits (although
we don't prevent this; maybe we should?).
However, it was a problem when you tried to reword a merge commit; this
previously failed with an error, as the test added in the previous commit
demonstrated.
Also, we want to add a new patch command that has to stop *before* the selected
commit (pull patch to new commit before the original one), and this wouldn't
work for the first commit in a feature branch, because it would have to set the
last commit before that to 'edit', which isn't possible if that's a merge (which
is likely).
To fix all this, use a 'break' before the selected commit if the commit is a
merge. It is important that we only do it in that case and not always, otherwise
we would break the new regression tests that were added a few commits ago.
I almost broke this during the development of this branch, so add a test to
guard against that. The point here is that the stack remains intact, i.e. the
newly created commit is the last commit of the lower branch, and thus shows the
"*".
I almost broke this during the development of this branch, so add a test to
guard against that. The point here is that the stack remains intact, i.e. the
renamed commit is the head of the lower branch, and thus shows the "*".
This is very old; I can only guess that this was added at a time where today's
list column handling wasn't in place yet, so the space was needed to separate
columns. This now causes a gap of two spaces between the rebase todo column and
the author column, which I'm sure wasn't intended. Funny that I never noticed.
Previously we would render the diff for a directory to the main/secondary pair,
but a diff for a file to the staging/stagingSecondary pair. (And similar for
commit files: main/secondary for directories, but
patchBuilding/patchBuildingSecondary for files.)
I always found this confusing and couldn't really understand why we are doing
this; but now it gets in my way because I want to attach a controller to
main/secondary so that they can be focused. So change it to always use the main
context pair for everything we render from a side panel.
When rerendering a view at the end of a refresh, we call HandleFocus only if the
view has the focus. This is so that we rerender the main view for the new
selection.
What was missing here is to update the view selection from the list selection if
the view doesn't have the focus, so that the selection is painted properly.
Normally this is not relevant because you don't see the selection if another
side panel has the focus; however, you do see it as an inactive selection when
e.g. a popup is shown, in which case it does matter.
This will become more important when we introduce section headers for commits,
because in that case the view selection needs to change when the working copy
state changes from normal to rebasing or vice versa, even if the list selection
stays the same.
The changed test submodule/reset.go shows how this was wrong before: when
entering the submodule again after resetting, there is a refresh which keeps the
same branch selected as before (master); however, since the branches panel is
not focused, the view didn't notice and kept thinking that the detached head is
selected (which it isn't, you can tell by running the test in sandbox mode and
focusing the branches panel at the end: you'll see that master is selected). So
the change in this commit fixes that.
We do this because
- it's closer to what you would do on the command line
- it simplifies the code a bit
- it will allow us to support cherry-picking merge commits.
We treat the .git/sequencer/todo file as read-only. Technically it seems it
would be possible to treat it as modifiable in the same way as
.git/rebase-merge/git-rebase-todo, effectively turning a cherry-pick or revert
that stops at a conflict into an interactive rebase; however, git itself doesn't
allow this (there is no "git cherry-pick --edit-todo"), so it seems safer not to
rely on it.
Theoretically it would be possible to allow modifying the rebase todos when a
cherry-pick or revert conflicts in the middle of a rebase. However, it would
introduce a bit of complexity to support this, as we would have to be able to
distinguish between rebasing todos and cherry-picking/reverting todos, which we
currently can't; it could also be a bit error-prone as far as edge cases are
concerned. And it's really a pretty uncommon situation, so it doesn't seem worth
it, and we just forbid all modifications to todos whenever we are cherry-picking
or reverting.
What happens here is that when stopping on an "edit" todo entry, we rely on the
assumption that if the .git/rebase-merge/amend file exists, the command was
successful, and if it doesn't, there was a conflict. The problem is that when
you stop on an edit command, and then run a multi-commit cherry-pick or rebase,
this will delete the amend file. You may or may not consider this a bug in git;
to work around it, we also check the existence of the rebase-merge/message file,
which will be deleted as well by the cherry-pick or revert.
This problem can't happen inside of lazygit itself right now, but this will
change in the future. It will only happen when you stopped in an interactive
rebase on an "edit" entry, and then you perform a revert or cherry-pick
consisting of more than one commit; in this situation lazygit will show a
conflict although there is none.
This is not possible with lazygit yet, as we don't support range-select for
reverting, and we don't use `git cherry-pick` for cherry-picking. Both will
change in the future, so it's good to fix this bug.
It is useful to see if the conflicted commit was a "pick" or an "edit". What's
more, we're about to add support for showing cherry-picks and reverts, and
seeing that a conflicted commit was a revert is important because its diff is
backwards compared to the diff of the conflicting files in the Files panel.
When you are in the middle of a rebase, and you cherry-pick a commit which
conflicts, it helps to be clear on whether you are prompted to continue the
cherry-pick or the rebase.
The situation is that you perform a rebase, and one of the commits becomes empty
during the rebase, in a way that couldn't be predicted before the rebase
started. To explain: git rebase has some logic where it immediately discards
commits if it can tell that they already exist in the branch being rebased onto;
it does this by looking at the patch ids of the commits to work out which
commits already exist upstream. But for those commits that become empty even
though there isn't a corresponding commit upstream, git stops with an error, and
lazygit detects this (in CheckMergeOrRebaseWithRefreshOptions) and automatically
continues the rebase.
This all works fine; I'm adding this test because I almost broke this during
development of this branch, so I'm adding it to guard against regressions.
When debugging an integration test that involves some behind-the-scenes
rebasing, the daemon lazygit would also wait for a debugger to attach. This is
very confusing because the test seems to hang; once you figured out what's going
on, it's inconvenient because you need to attach a debugger to the daemon every
time you debug the test.
Now, it would sometimes be useful to be able to debug the daemon itself (whether
inside an integration test, or during normal usage), and I have often wished to
be able to do that. We might introduce an additional env var (and command-line
option?) to enable this; but that's out of scope here.
For #, !, [, and ], the problem is that it doesn't ignore the file because the
special characters need to be quoted. For *, the problem is that it ignores too
much (it also hides the abc_def file because the * is treated as a glob).
