This solves several problems that arise from opening a menu while the prompt is
open. We might try to solve these in a different way, e.g. by dismissing the
search prompt before opening a menu, but restricting what you can do while the
prompt is open seems like the more robust fix.
To achieve this, we
- call resetKeyBindings both when opening and when closing the search/filter
prompt
- change the keybindings to only contain the ones for the search prompt when
that context is active.
So far, lazygit has always auto-staged files as soon as the conflict markers
disappeared from them, which means that we could rely on any file that still had
a status of "UU" to still contain conflict markers.
We are going to make the auto-staging optional in the next commit, and in that
case the user will want to manually stage "UU" files; so we must now check
whether the file contains conflict markers, and disallow the staging in that
case.
We make the name of the GetSelectedRefRangeForDiffFiles very specific on purpose
to make it clear that this is only for switching to diff files, so the
implementations can make assumptions about that (unlike GetSelectedRef, which is
used for different purposes and needs to stay more generic).
Right now it doesn't do very much yet, but it's still worth it even in this
state, I'd say. The function is going to become a bit longer in the next commit,
though.
In other views that show lists of commits (reflog and stash) it doesn't make
sense to show a range diff of selected entries because they don't form a linear
sequence, so we keep the previous behavior of showing the diff for the free end
of the selection range in those view.
The same applies to the commits view if the selection range includes rebasing
todos; these can have an arbitrary order, and a range diff doesn't make sense
for those.
Hopefully this will help alleviate the problem that diffing mode is sticky, and
you often forget that it's still on.
Make it magenta like the mode text in the information view.
This fixes two problems:
- Set the title ref to the commit description (like
SwitchToDiffFilesController.enter does), rather than just the hash
- SetTitleRef only sets the title of the DynamicTitleBuilder, but doesn't set it
on the view; this only happens in ContextMgr.Activate, so you'd have to switch
to a different side panel and then back to see the new title.
The three nested `if` statements may looks strange, and you might wonder why we
don't have single one with &&. The answer is that later in this branch we will
add an `else` block to the middle one.
In the next commit it will be reused for checking whether we need to reset the
patch; and extracting it makes it easier to extend it for non-sticky diff ranges
later in the branch.
viewFiles is only called from enter; it doesn't make much sense to fill in a
SwitchToCommitFilesContextOpts struct to pass it to viewFiles for this one call.
Simply inline viewFiles into enter and get rid of all that.
I found this indirection confusing when reading the code. It looks like
SwitchToDiffFilesController is instantiated with different such contexts, but
it's always Contexts.CommitFiles, so just use that directly.
Note that this doesn't only affect the diff views, which are the ones where we
want this, but also the status view, where the plan was to keep them underlined
always for better discoverability. We could make this configurable dynamically
(by adding another flag to ViewUpdateOpts), but actually I think it's fine this
way.
Any newly loaded custom command coming from the per-repo config file should add
to the global ones (or override an existing one in the global one), rather than
replace all global ones.
We can achieve this by simply prepending the newly loaded commands to the
existing ones. We don't have to take care of removing duplicate key assignments;
it is already possible to add two custom commands with the same key to the
global config file, the first one wins.
If a `t.FileSystem().FileContent("file.txt", Equals("bla"))` assertion fails
because the file doesn't exist, the error would say
Expected path 'file.txt' to not exist, but it does
which is very confusing.
We fixed one specific scenario where this happened ealier in this branch, but in
case there are more that we don't know about yet, at least make sure we don't
crash.
I don't know what this condition is supposed to guard against, or whether we
really need it (it was added in 06ca71e955, and the commit message of that
commit only says "fix bug"). But if we do need it, then it seems that `>=` is
more correct than `>`.
It is already possible to search a filtered list by searching first, and then
enabling a filter, so I found it inconsistent to not allow searching when you
are already filtering. One reason for not allowing this might be that the search
status (on the left) hides the filter status (on the right), but if we think
that's enough reason to not allow both at the same time, then we should cancel a
search when we enter filtering.
There are many situations where this can arise. Some examples are:
- the terminal window is small, and you are showing a view that shows more
content than fits into the view port, and the view is scrolled all the way
down; now you resize the terminal window to a taller size. Previously, the
scroll position of the view would stay the same, so it would add blank space
at the bottom; now it will scroll to fill that blank space with content
- expandFocusedSidePanel is on, you go to the bottom of a list view, now switch
to a different panel, then scroll that (now unfocused) panel all the way down
with the scroll wheel; now you focus that panel again. It becomes larger
because of the accordion behavior, but would show blank space at the bottom.
And probably others that I can't remember right now. I only remember that I
always found it confusing to look at a view that had blank space at the bottom
even though it had more content to scroll into view.
