This in itself is not an improvement, because hashes are unique (they are shared
between real commits and rebase todos, but there are so few of those that it
doesn't matter). However, it becomes an improvement once we also store parent
hashes in the same pool; but the real motivation for this change is to also
reuse the hash pointers in Pipe objects later in the branch. This will be a big
win because in a merge-heavy git repo there are many more Pipe instances than
commits.
The "// merge commit" comment was plain wrong, this is any commit that has a
parent, merge or not. The "else if" condition was unnecessary, a plain "else"
would have been enough. But the code in the two blocks was almost identical, so
extract the one thing that was different and unify it.
And while we're at it, use IsFirstCommit() instead of counting parents.
This makes it easier to copy diff hunks and paste them into code. We only strip
the prefixes if the copied lines are either all '+' or all '-' (possibly
including context lines), otherwise we keep them. We also keep them when parts
of a hunk header is included in the selection; this is useful for copying a diff
hunk and pasting it into a github comment, for example.
A not-quite-correct edge case is when you select the '--- a/file.txt' line of a
diff header on its own; in this case we copy it as '-- a/file.txt' (same for the
'+++' line). This is probably uncommon enough that it's not worth fixing (it's
not trivial to fix because we don't know that we're in a header).
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.
This is very similar to what we are doing for staging or discarding hunks in the
Files panel. Git doesn't allow applying patches with a zero context size (unless
you use the --unidiff-zero option, which is discouraged).
The long story: I want to call this function from RefsHelper; however, I can't
make WorkingTreeHelper a field of RefsHelper because RefsHelper is already a
field in WorkingTreeHelper, so that would be a circular dependency.
The shorter story: there's really little reason to have to instantiate a helper
object in order to call a simple function like this. Long term I would like to
get to a state where a lot more of these helper functions are free-standing, and
you pass in the data they need.
While at it, simplify the implementation of AnyStagedFiles and AnyTrackedFiles
to one-liners.
It's the same, really, except that GetCheckedOutRef() does a check if any
branches exist and returns nil if not. Since we are accessing the returned
branch unconditionally without checking for nil, it seems this check is not
needed here. (The functions we are touching here are called from handlers that
are guarded with itemSelected or singleItemSelected, so we know that at least
one branch exists.)
The goal is to get rid of the dependency to refsHelper.
In this commit this is only possible by pressing '0' in a side panel; we'll add
mouse clicking later in the branch.
Also, you can't really do anything in the focused view except press escape to
leave it again. We'll add some more functionality in a following commit.
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.
Previously we would create new Commit objects to store in the CherryPicking mode
which only contained a name and hash, all other fields were unset. I'm not sure
why we did this; it's easier to just reference the original commits. Later on
this branch we will need this because we need to determine whether a copied
commit was a merge commit (by looking at its Parents field).
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.
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.
This is equivalent in the current state of the code, but it will no longer be
after the next commit, because we will introduce a new status value
StatusConflicted. And in a later PR we might add yet another value
StatusCherryPicking to distinguish rebase todos from cherry-pick todos; using
commit.IsTODO is a safer way to check whether a commit is any of these.
