The rendering of remote branches is in no way dependent on the width of the view
(or the screen mode). Unlike in the local branches view, we don't truncate long
branch names here (because there's no more information after them).
This is an error introduced in d5b4f7bb3e.
For checkboxes it probably doesn't really make sense to use them yet, because
we'd have to find a way how you can toggle them without closing the dialog; but
we already provide rendering for them to lay the ground.
But radio buttons can be used already, because for those it is ok to close the
dialog when choosing a different option (as long as there is only one grounp of
radio buttons in the panel, that is).
Strike it through if not applicable. This will hopefully help with confusion
about the meaning of "all" in the "Discard all changes" entry; some people
misunderstand this to mean all changes in the working copy. Seeing the "Discard
unstaged changes" item next to it hopefully makes it clearer that "all" is meant
in contrast to that.
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.
We're going to add another argument in the next commit, and that's getting a bit
much, especially when most of the arguments are bool and you only see true and
false at the call sites without knowing what they mean.
This is important when using a pager that draws a horizontal line across the
entire width of the view; when changing from a file or directory that has only
unstaged (or only staged) changes to one that has both, the main view is split
in half, but the PTY task would be run on the view in its old state, so the
horizonal line would be too long and wrap around.
All PTYs were created with the size of the main view, on the assumption that
main and secondary always have the same size. That's not true though; in
horizontal split mode, the width of the two views can differ by one because of
rounding, and when using a pager that draws a horizontal line across the width
of the view, this is visible and looks very ugly.
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.
When refreshViewportOnChange is true, we would refresh the viewport once at the
end of FocusLine, and then we would check at the end of AfterLayout if the
origin has changed, and refresh again if so. That's unnecessarily complicated,
let's just unconditionally refresh at the end of AfterLayout only.
We want to add an additional method to ISearchableContext later in this branch,
and this will make sure that we don't forget to implement it in any concrete
context.
Searching in the "Divergence from upstream" view would select the wrong lines.
The OnSearchSelect function gets passed a view index, and uses it to select a
model line. In most views these are the same, but not in the divergence view
(because of the Remote/Local section headers).
ListContextTrait.OnSearchSelect was introduced in 138be04e65, but it was never
called. I can only guess that a planned refactoring wasn't finished here.
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.
Copy the slice into a variable and use that throughout the whole operation; this
makes us a little more robust against the model refreshing concurrently.
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.
Instead, disable the individual entries in the menu.
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.
We aren't using them, yet, except for deciding whether to show the warning about
hunks with only added lines.
Add a bit of test coverage for parseDiff while we're at it.