Previously, the feedback you got when pressing "-" was just a "Checking out..."
status in the bottom line. This was both easy to miss if you are used to looking
for an inline status in the branches panel, and it didn't provide information
about which branch was being checked out, which can be annoying in very large
repos where checking out takes a while, and you only see at the end if you are
now on the right branch.
Improve this by trying to figure out which branch was the previously checked out
one, and then checking it out normally so that you get an inline status next to
it (as if you had pressed space on it). There are cases where this fails, e.g.
when the previously checked out ref was a detached head, in which case we fall
back to the previous behavior.
In all other menus besides the keybindings menu it makes sense to hide
keybindings that match the confirmMenu binding. This is important to make it
clear which action will be triggered when you press the key.
In the keybindings menu this is different; the main purpose of that menu is not
to allow triggering commands by their key while the menu is open, but to serve
as a reference for what the keybindings are when it is not open. Because of
this, it is more important to show all bindings in this menu, even if they
conflict with the confirmMenu key.
This fixes a regression introduced in b3a3410a1a.
It seems useful to have the flexibility to remap "enter" in confirmations to
"y", but keep "enter" for menus and suggestions (even though we sometimes use
menus as confirmations, but it's still good to give users the choice).
The universal.confirm keybinding is the wrong one to use for this, we want
universal.goInto instead. They are both bound to "enter" by default, but when
remapping confirm to "y" we don't want to use that for entering worktrees.
Rebinding the universal.confirm keybinding currently doesn't make sense, because
the rebound key would also be used for editable prompts, which means you would
only be able to bind it to a ctrl key (not "y", which is desirable for some
people), and also it would allow you to enter a line feed in a branch name.
Fix this by always using enter for editable prompts.
So far, confirmations and prompts were handled by the same view, context, and
controller, with a bunch of conditional code based on whether the view is
editable. This was more or less ok so far, since it does save a little bit of
code duplication; however, now we need separate views, because we don't have
dynamic keybindings, but we want to map "confirm" to different keys in
confirmations (the "universal.confirm" user config) and prompts (hard-coded to
enter, because it doesn't make sense to customize it there).
It also allows us to get rid of the conditional code, which is a nice benefit;
and the code duplication is actually not *that* bad.
To fix the problem described in the previous commit, iterate backwards over the
stashes that we want to delete. This allows us to use their Index field.
In some cases we set a disabled reason but leave the text empty, so that we
don't get an error toast when the item is invoked. In such a case it looks
awkward if there is a tooltip showing "Disabled: " with no following text.
For many menus, just "Close" is fine, but some menus act more like confirmations
(e.g. the menu that appears when you cherry-pick and get conflicts); in this
case, it's good to make it more obvious that hitting esc cancels the whole
thing.
Dismissing a range selection is handled by the global escape handler for all
list views, but not for the staging and patch building views, so we need to make
the esc description dynamic for these, too.
The main reason for this is that sometimes the escape key is handled by a local
binding, in which case it appears before the '?' binding in the options status
bar, and other times it is handled by the global controller, in which case it
appeared after. Moving it to before the keybindings menu handler makes it appear
before '?' in both cases.
Also, if the window is too narrow to show all keybindings, the ones that don't
fit will be truncated, and in this case it is more important to show the esc
binding because of its context sensitivity.
This also moves the esc entry up a few positions in the keybindings menu, but I
don't think this matters much.
It's maybe not totally obvious, so let's show it.
Esc now appears twice in the status bar, because the global controller also
appends its generic "Cancel". We'll fix this in the next commit.
When showing a confirmation whose text ended with a line feed, we would make the
popup panel one line less tall than it needs to be, so it would show a scroll
bar. One example where this occurred is the very first popup that users ever see
(the "seriously you rock" message for new users); that's a pretty bad first
impression.
This happens because our code to suppress trailing newlines in views doesn't
work for styled text, and the text in confirmations is bold. This code checks if
the last character of the text is a line feed, and in this case it isn't; the
escape sequence for turning bold off comes after it.
We should really fix this by improving that mechanism, but this would require
some tricky logic, so as a quick fix, trim trailing (and leading) linefeeds from
the text that we display in a confirmation, before making it bold.
When the useHunkModeInStagingView config is on and you enter the staging view
with hunk selection enabled, it is confusing to see "a: Select hunk" in the
options view at the bottom.
This makes it easier to use the full ref in the git merge-base call, which
avoids ambiguities when there's a tag with the same name as the current branch.
This fixes a hash coloring bug in the local commits panel when there's a tag
with the same name as the checked out branch; in this case all commit hashes
that should be yellow were painted as red.
Also, fix two other commands that stage all files under the hood:
- when continuing a rebase after resolving conflicts, we auto-stage all files,
but in this case we never want to include untracked files, regardless of the
filter
- likewise, pressing ctrl-f to find a base commit for fixup stages all files for
convenience, but again, this should only stage files that are already tracked
Storing it in the Tag struct makes loading tags a lot slower when there is a
very large number of tags; so determine it on the fly instead. On my machine,
the additional call takes under 5ms, so it seems we can afford it.
There's no reason not to allow these.
Technically we could enable a few more, but I chose not to because some might be
surprising or confusing in filtering mode. For example, creating a fixup commit
would work (shift-F), but the newly created commit might not show up if it
doesn't match the filter. Similarly, pressing `f` to fixup a commit into its
parent would work, but that parent commit might not be visible, so users might
expect to be fixing up into the next visible commit.
This is not a behavior change, we already include these in the menu, but that's
because of a bug that we will fix in the next commit.
I find it useful to see these commands, especially for rarely-used custom
commands that you don't want to waste a keybinding on.
When filtering for a file path we use the --follow option for "git log", so it
will follow renames of the file, which is great. However, if you then selected
one of the commits before a rename, you didn't see a diff, because we would pass
the original filter path to the "git show" call.
To fix this, call git log with the --name-status option when filtering by path,
so that each commit reports which file paths are touched in this commit;
remember these in the commit object, so that we can pass them to the "git show"
call later.
Be careful not to store too many such paths unnecessarily. When filtering by
folder rather than file, all these paths will necessarily be inside the original
filter path, so detect this and don't store them in this case.
There is some unfortunate code duplication between loading commits and loading
reflog commits, which I am too lazy to clean up right now.
I don't know why this function argument was added, but I don't like unnecessary
indirections, so I'm removing it as SubCommitsHelper has access to everything it
needs to do it itself.
Recycle reflog commits only for the non-filtered ones.
If we're not filtering, FilteredReflogCommits and ReflogCommits are the same. If
we then enter filtering and get reflog entries again, and pass a
lastReflogCommit, we'd recycle the previous FilteredReflogCommits, which are
unfiltered, so that's no good. Work around this by simply never using the
recycle mechanism when getting the filtered reflog commits.
We could do better by remembering what the last filter path or author was, and
only suppressing the recycling when it changed; but that's more complexity than
I want to add, so hopefully this is good enough.
Using the filtered one is probably not a good idea. It didn't do much harm
because the split of ReflogCommits and FilteredReflogCommits doesn't really work
right now (FilteredReflogCommits is always the same as ReflogCommits, even in
filtering mode), but we'll fix this in the next commit.
I can only guess what happened here: in aa750c0819, this code to manually load
the reflog commits was added, to make sorting branches by recency work when the
reflog is filtered by path. At that time we didn't have separate ReflogCommits
and FilteredReflogCommits models yet. Then, FilteredReflogCommits was introduced
(in 8822c409e2), probably for the very purpose of being able to get rid of this
again; but then it was forgotton to actually get rid of it.
The funny thing is that the introduction of FilteredReflogCommits happened in
the very next commit, 15 minutes later.
Since filtering switches to half-screen mode in the local commits panel, most
people probably didn't notice, but we do also filter those other views. So when
leaving half-screen mode (but not filtering), you could switch to sub-commits or
stashes, and those would show the filtered view only after the next refresh
(e.g. after a background fetch). It's worse when leaving filtering, because this
goes back to normal screen mode, and you would often see an empty stashes panel
after that (until the next background fetch), which is quite confusing.
I also find it questionable to always switch focus to the commits panel when
entering filtering. If it is initiated from subcommits, reflog, or stashes,
maybe we want to stay there. I'm not changing this now since I'm unsure how much
people rely on the current behavior.
When exiting filtering mode while the focus is not in the local commits panel,
the call to HandleFocus would render the selection in the commits panel as if
the panel had the focus.
The call to HandleFocus was introduced recently in 4981419ba9 in an attempt to
rerender the main view correctly, but it should only have been called when local
commits is the focused context. But instead, we can use PostRefreshUpdate, which
handles this distinction.
