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.
The test shows that we are currently auto-forwarding branches even if they are
checked out by another worktree; this is quite bad, because when you switch to
that other worktree you'll see that the files that are touched by the fetched
commits are all modified (which we don't test here).
This is similar to using lazygit's Git.Paging.ExternalDiffCommand config, except
that the command is configured in git. This can be done either with git's
`diff.external` config, or through .gitattributes, so it gives a bit more
flexibility.
Many people don't understand what this means, which is apparent from the amount
of issues that got filed because of this. Let's get rid of it to avoid this
confusion. People will have to configure their pager twice if they want to use
it both on the command line and in lazygit, which I think is not a big deal.
This was needed in an earlier version of the test, when we asserted the file
content in a more complicated way. It should have been removed in caca62b89e.
This frees up ctrl-z for suspend. Hopefully, redo is not such a frequently used
operation that the change annoys people.
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
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.
Sometimes there is a local and a global keybinding for the same key; if both are
configured to be shown in the options map, we should only show the local one
because it takes precedence. This happens for example for <esc> in a popup, or
for <esc> in the focused main view.
Note that this is also a problem in the keybindings menu, and we don't solve
that here.
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.
An example for this can be seen in the next commit.
We make this a setter rather than an added constructor argument so that we don't
have to change all those contexts that don't want to make use of this.
This assertion didn't test anything useful, given that the filtered view already
has two lines. This should have been adapted in 9f0b4d0000 when we added
section headers while filtering.
The previous commit already fixed the user-visible lag, but there's still a
problem with multiple background git processes consuming resources calculating
diffs that we are never going to show. Improve this by terminating those
processes (by sending them a TERM signal).
Unfortunately this is only possible on Linux and Mac, so Windows users will have
to live with the higher CPU usage. The recommended workaround is to not use
"diff.algorithm = histogram".
One reason why git diff can be very slow is when "diff.algorithm = histogram" is
being used. In this case, showing a very long single-file diff can take seconds
to load, and you'll see the "loading..." message in the main view until we got
the first lines of the diff to show. There's nothing really we can do about this
delay; however, when switching to another, shorter file (or commit) while the
"loading..." message is still showing, this switch should be instantaneous. And
it was before 0.54.0, but we broke this in 0.54.0 with 8d7740a5ac (#4782); now
users have to wait for the slow git diff command to output more text before the
switch occurs.
To fix this, don't block waiting for the process to terminate if we just stopped
it.
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.
