Several custom patch commands on parts of an added file would fail with the
confusing error message "error: new file XXX depends on old contents". These
were dropping the custom patch from the original commit, moving the patch to a
new commit, moving it to a later commit, or moving it to the index.
We fix this by converting the patch header from an added file to a diff against
an empty file. We do this not just for the purpose of applying the patch, but
also for rendering it and copying it to the clip board. I'm not sure it matters
much in these cases, but it does feel more correct for a filtered patch to be
presented this way.
This currently works (albeit with a bit of manual work, as the user needs to
resolve conflicts), and we add this test just to make sure that we don't break
it with the following change.
This currently works, we add it as a regression test to make sure we don't break
it. It is an interesting test because it turns the deletion of the file in the
moved-from commit into a modification.
When switching to a repo that was open before, the context tree is reused, so
before adding keybinding functions to those contexts again, we need to clear the
old ones.
Unfortunately it isn't possible to delete them. This would often be useful, but
our todo rewriting mechanisms rely on being able to find todos by some
identifier (hash for pick, ref for update-ref), and exec todos don't have a
unique identifier.
If exactly one candidate from inside the current branch is found, we return that
one even if there are also hunks belonging to master commits; we disregard those
in this case.
It has two modified hunks, one for a master commit and one for a branch commit.
Currently we get an error mentioning those two commits, but we would like to
silently select the branch commit.
Put it into the individual menu items instead.
Again, this is necessary because we are going to add another entry to the menu
that is independent of the selected branch.
Previously the entire status was colored in a single color, so the API made
sense. This is going to change in the next commit, so now we must include the
color in the string returned from BranchStatus(), which means that callers who
need to do hit detection or measure the length need to decolorize it.
While we're at it, switch the order of ↑3↓7 to ↓7↑3. For some reason that I
can't really explain I find it more logical this way. The software out there is
pretty undecided about it, it seems: VS Code puts ↓7 first, and so does the
shell prompt that comes with git; git status and git branch -v put "ahead" first
though. Shrug.
Use Equals instead of Contains for asserting the status view content. This
solves the problem that we might assert Contains("↓2 repo"), but what it really
shows is "↑1↓2 repo", and the test still succeeds. At best this is confusing.
Also, this way we don't have to use the awkward DoesNotContain to check that it
really doesn't show a checkmark.
To do this, we need to fix two whitespace problems:
- there was always a space at the end for no reason. Simply remove it. It was
added in efb51eee96, but from looking at that diff it seems it was added
accidentally.
- there was a space at the beginning if the branch status was empty. This is
actually a cosmetic problem, for branches without a status the text was
indented by once space. Change this so that the space is added conditionally.
It's a bit awkward that we have to use Decolorise here, but this will go away
again later in this branch.
It can optionally be used to set the title of the panel that shows the output of
a command (when showOutput is true). If left unset, the command string is used
as the title.
To determine whether we need to ask for force pushing, we need to query the push
branch rather than the upstream branch, in case they are not the same.
Our code doesn't realize that we need to prompt the user to force push, when the
branch is up-to-date with its upstream but not with the branch that we're
pushing to.
It is unexpected that a function called PushBranch also sets the upstream
branch; also, we want to add a PushBranch function in the next commit that
doesn't.
In go 1.22, loop variables are redeclared with each iteration of the
loop, rather than simple updated on each iteration. This means that we
no longer need to manually redeclare variables when they're closed over
by a function.
For custom commands it is useful to select an earlier command and have it copied
to the prompt for further editing. This can be done by hitting 'e' now.
For other types of suggestion panels we don't enable this behavior, as you can't
create arbitrary new items there that don't already exist as a suggestion.
In the custom commands panel you can now tab to the suggestions and hit 'd' to
delete items from there. Useful if you mistyped a command and don't want it to
appear in your history any more.
The rebase.updateRefs feature of git is very useful to rebase a stack of
branches and keep everything nicely stacked; however, it is usually in the way
when you make a copy of a branch and want to rebase it "away" from the original
branch in some way or other. For example, the original branch might sit on main,
and you want to rebase the copy onto devel to see if things still compile there.
Or you want to do some heavy history rewriting experiments on the copy, but keep
the original branch in case the experiments fail. Or you want to split a branch
in two because it contains two unrelated sets of changes; so you make a copy,
and drop half of the commits from the copy, then check out the original branch
and drop the other half of the commits from it.
In all these cases, git's updateRefs feature insists on moving the original
branch along with the copy in the first rebase that you make on the copy. I
think this is a bug in git, it should create update-ref todos only for branches
that point into the middle of your branch (because only then do they form a
stack), not when they point at the head (because then it's a copy). I had a long
discussion about this on the git mailing list [1], but people either don't agree
or don't care enough.
So we fix this on our side: whenever we start a rebase for whatever reason, be
it interactive, non-interactive, or behind-the-scenes, we drop any update-ref
todos that are at the very top of the todo list, which fixes all the
above-mentioned scenarios nicely.
I will admit that there's one scenario where git's behavior is the desired one,
and the fix in this PR makes it worse: when you create a new branch off of an
existing one, with the intention of creating a stack of branches, but before you
make the first commit on the new branch you realize some problem with the first
branch (e.g. a commit that needs to be reworded or dropped). It this case you do
want both branches to be affected by the change. In my experience this scenario
is much rarer than the other ones that I described above, and it's also much
easier to recover from: just check out the other branch again and hard-reset it
to the rebased one.
[1]
https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/
We upgraded our minimum Go version to 1.21 in commit
57ac9c2189458a7f0e63c2e9cac8334694a3d545. We can now replace our
`utils.Min` and `utils.Max` functions with the built-in `min` and `max`.
Reference: https://go.dev/ref/spec#Min_and_max
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Using the "Add to .git/info/exclude" in a worktree results in an error message,
as the test shows. The same would happen in a submodule, but I'm not adding an
extra test for that, as the circumstances are the same.
When exiting filtering mode, we currently keep the selection index the same in
the commits panel. This doesn't make sense at all, since the index in the
filtered view has no relation to the index in the unfiltered view.
I often use filtering mode (either by path or by author) to find a given commit
faster than I would otherwise be able to. When exiting filtering mode, it's
useful to keep the same commit selected, so that I can look at the surrounding
commits, see which branch it was a part of, etc. So reselect the commit again
after exiting filtering mode.
Sometimes this is not possible, most likely when the commit is so long ago that
it's outside of the initial 300 range. In that case, at least select the commit
again that was selected before I entered filtering; this is still better than
arbitrarily keeping the same selection index.
It would crash when some keybindings are set to null, and the filter string is
such that only those keybindings remain visible.
The reason for the crash is that when inserting non-model items (menu section
headers in this case) you specify a column to align them to. This works on the
assumption that the number of columns is always the same. It can cope with the
case that columns are removed because they are empty for all items; but it can't
cope with the case that the getDisplayStrings function returns a lower number of
columns.
And this is what happened here: MenuViewModel.GetDisplayStrings would omit the
keybinding column when none of the entries have a keybinding. This logic is
unnecessary, the generic list rendering mechanism takes care of this, so
removing this logic fixes the crash.
We do have to make sure though that the column is really empty when there's no
keybinding, so change the logic to use FgCyan only when there's a keybinding.
It is a bad idea to read a git-rebase-todo file, remove some update-ref todos,
and write it back out behind git's back. This will cause git to actually remove
the branches referenced by those update-ref todos when the rebase is continued.
The reason is that git remembers the refs affected by update-ref todos at the
beginning of the rebase, and remembers information about them in the file
.git/rebase-merge/update-refs. Then, whenever the user performs a "git rebase
--edit-todo" command, it updates that file based on whether update-ref todos
were added or removed by that edit. If we rewrite the git-rebase-todo file
behind git's back, this updating doesn't happen.
Fix this by not updating the git-rebase-todo file directly in this case, but
performing a "git rebase --edit-todo" command where we set ourselves as the
editor and change the file in there. This makes git update the bookkeeping
information properly.
Ideally we would use this method for all cases where we change the
git-rebase-todo file (e.g. moving todos up/down, or changing the type of a
todo); this would be cleaner because we wouldn't mess with git's private
implementation details. I tried this, but unfortunately it isn't fast enough.
Right now, moving a todo up or down takes between 1 and 2ms on my machine;
changing it to do a "git rebase --edit-todo" slows it down to over 100ms, which
is unacceptable.
In the test we simply removed the update-ref todo but didn't make any other
changes to the todos. This should really have kept everything the way it was,
including the other branch head. The fact that the star was gone was really
because of the bug that we are going to fix later in the branch.
Change the test so that it also makes a change before the update-ref todo; this
way we test that the star is gone because we deleted the update-ref, not because
of the bug.
To guard against the bug, we add another assertion for the branches view to test
that both branches are still there. This currently fails.
