The row is reserved for as long as such a menu is open, even while it is
still hidden, so that it can appear without moving the menu. That costs
two rows of the popup, which is why the screen has to be a little taller
before a menu is worth showing at all.
The prompt in front of the input field is dropped when the row gets too
narrow to type in, and the keybindings menu says what '@' does when it
still fits.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
PopupMutex guarded CurrentPopupOpts against a popup being created on a
worker goroutine while the UI thread deactivated it, or reset it on a
repo switch. Now that popup and menu creation is bounced onto the UI
thread, every access to CurrentPopupOpts — create, deactivate, and the
reset-on-switch (which already runs on the UI thread) — happens on the
one goroutine, so the mutex protects nothing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
I got confused when I investigated the bug that was fixed in the previous
commit, so hopefully these will be helpful for the next person reading this
code.
The symptom of the bug was that confirmation panels with wrapped text were one
character too wide, which could sometimes result in them being one line too
heigh. One concrete example is the "Discard file changes" confirmation that
appears when pressing `d` in the commit files panel.
I got very confused when investigating this bug, and the reason is that
getPopupPanelDimensionsAux took two parameters panelWidth and panelHeight, but
the second was actually contentHeight. Rename it to contentHeight, and change
the first one to also be the content width rather than the panel width. This is
easier for clients, and less confusing.
On very large screens they would get ridiculously wide.
The limits we chose here are a little arbitrary:
- 80 for confirmations and prompts
- 90 for menus
- auto-wrap width plus 25 for the commit message editor (or 100 if auto-wrap is
off)
This doesn't really solve a pressing problem, because I guess it's unlikely that
users add spaces at the beginning or end of what they type into a prompt; but it
could happen, and in this case we almost always want to strip it. Just adding
this here for completeness while I was working on this code.
The only exception is the input prompt of custom commands, because who knows
what users want to use that input for in their custom command.
Most of our prompts don't (shouldn't) allow empty input, but most callers didn't
check, and would run into cryptic errors when the user pressed enter at an empty
prompt (e.g. when creating a new branch). Now we simply don't allow hitting
enter in this case, and show an error toast instead.
This behavior is opt-out, because there are a few cases where empty input is
supported (e.g. creating a stash).
Previously it was used both for the Confirm handler and the Cancel handler, as
well as for the Confirm handler of confirmation popups (not prompts). There was
no other way to do it given how wrappedConfirmationFunction was shared between
all these; but now there is. The logic is really only needed for the Confirm
handler of prompts.
This doesn't fix anything, it just makes things clearer.
And call this new helper function from both wrappedConfirmationFunction and
wrappedPromptConfirmationFunction; this gives us more flexibility to do
different things in each of those.
This is likely to do bad things; for example, if the prompt is the shell command
prompt, then we would run into what looks like a deadlock bug in tcell. In other
cases, the characters in the following lines might be treated as random commands
after the prompt is confirmed.
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.
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.
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.
Apparently this was an attempt at working around go's lack of default arguments,
but it's very unidiomatic and a bit confusing. Make it a normal parameter
instead, so all clients have to pass it explicitly.
For editable views it is important to actually show the blank line so that we
can put the cursor there for typing.
This fixes problems with adding blank lines at the end of longer commit
messages.
to make it more generally usable by clients other than ConfirmationHelper, which
we will do later in this branch. Rename it to WrapViewLinesToWidth while we're
at it.
Add tests; in particular, add a sanity check that we wrap lines the same way as
gocui does. The tests that are added here are the same ones as in gocui for its
lineWrap function, but we'll extend them a bit in later commits in this branch.
Original commit message of the gocui change:
This fixes View.Size, Width and Height to be the correct (outer) size of a view
including its frame, and InnerSize/InnerWidth/InnerHeight to be the usable
client area exluding the frame. Previously, Size was actually the InnerSize (and
a lot of client code used it as such, so these need to be changed to InnerSize).
InnerSize, on the other hand, was *one* less than Size (not two, as you would
have expected), and in many cases this was made up for at call sites by adding 1
(e.g. in calcRealScrollbarStartEnd, parseInput, and many other places in the
lazygit code).
There are still some weird things left that I didn't address here:
- a view's lower-right coordinates (x1/y1) are one less than you would expect.
For example, a view with a 2x2 client area like this:
╭──╮
│ab│
│cd│
╰──╯
in the top-left corner of the screen (x0 and y0 both zero) has x1/xy at 3, not
4 as would be more natural.
- a view without a frame has its coordinates extended by 1 on all sides; to
illustrate, the same 2x2 view as before but without a frame, sitting in the
top-left corder of the screen, has coordinates x0=-1, y0=-1, x1=2, y1=2. This
is highly confusing and unexpected.
I left these as they are because they would be even more of a breaking change,
and also because they don't have quite as much of an impact on general app code.
Probably not the most import feature in the world, but when resizing the
terminal window while multiple popup panels were open at the same time, we would
only resize the topmost one.
The main reason for changing this is because it makes the next commit easier to
implement.
We forgot to handle the "suggestions" and "commitDescription" view names.
Instead of listing all the names of views that can appear in popups though,
let's use the context kind for this, which feels more robust.
This is a change in behavior: previously, clicking outside of the search or
filter prompt would close the prompt, now it no longer does (because search has
a persistent popup kind, but it wasn't listed in the list of view names before).
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.
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.
