runewidth.StringWidth is an expensive call, even if the input string is pure
ASCII. Improve this by providing a wrapper that short-circuits the call to len
if the input is ASCII.
Benchmark results show that for non-ASCII strings it makes no noticable
difference, but for ASCII strings it provides a more than 200x speedup.
BenchmarkStringWidthAsciiOriginal-10 718135 1637 ns/op
BenchmarkStringWidthAsciiOptimized-10 159197538 7.545 ns/op
BenchmarkStringWidthNonAsciiOriginal-10 486290 2391 ns/op
BenchmarkStringWidthNonAsciiOptimized-10 502286 2383 ns/op
In d5b4f7bb3e and 58a83b0862 we introduced a combined mechanism for rerendering
views when either their width changes (needed for the branches view which
truncates long branch names), or the screen mode (needed for those views that
display more information in half or full screen mode, e.g. the commits view).
This was a bad idea, because it unnecessarily rerenders too many views when just
their width changes, which causes a noticable lag. This is a problem, for
example, when selecting a file in the files panel that has only unstaged
changes, and then going to one that has both staged and unstaged changes; this
splits the main view, causing the side panels to become a bit narrower, and
rerendering all those views took almost 500ms on my machine. Another similar
example is entering or leaving staging mode.
Fix this by being more specific about which views need rerendering under what
conditions; this improves the time it takes to rerender in the above scenarios
from 450-500s down to about 20ms.
This reintroduces the code that was removed in 58a83b0862, but in a slightly
different way.
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.
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.
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.
This broke with 81b497d186 (#3387). In that PR I claimed that we never want to
ask for force-pushing if the server rejected the update, on the assumption that
this can only happen because the remote tracking branch is not up to date, and
users should just fetch in this case. However, I didn't realize it's even
possible to have a branch whose upstream branch is not stored locally; in this
case we can't tell ahead of time whether a force push is going to be necessary,
so we _have_ to rely on the server response to find out. But we only want to do
that in this specific case, so this is not quite an exact revert of 81b497d186.
When branches are sorted by recency we have this logic that first loads the
branches so that they can be rendered quickly; in parallel, it starts loading
the reflog in the background, and when that's done, it loads the branches again
so that they get their recency values. This means that branches are loaded twice
at startup.
We don't need this logic when branches are not sorted by recency, so we can
simply load branches and reflog in parallel like everything else.
This shouldn't change any user observable behavior, it just avoids doing
unnecessary work at startup.
For tooltips that are just one or two characters longer than the available
width, the last word would be cut off. On my screen this happened for the
tooltip for the fixup command.
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.
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.
Sometimes it takes a while to get PRs accepted upstream, and this blocks our
progress. Since I'm pretty much the only one making changes there anyway, it
makes sense to point to my fork directly.
