Replace merge-tool with merge options menu that allows resolving all
conflicts for selected files as ours, theirs, or union, while still
providing access to the merge tool.
The root item's path is ".", and the path of a file at top level is "./file".
When using GetPath, this gives us "." and "file", respectively, and
isDescendentOfSelectedNodes would return false for these.
Working with the internal paths (i.e. without stripping the leading "./") fixes
this.
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.
Previously a click was detected as a double-click whenever the click was on the
already selected line, regardless of how long ago the last click was (or even
when it wasn't selected by clicking at all). Now that gocui supports proper
double-click detection, we can do better.
When entering filtering we would only call FocusLine, which takes care of
highlighting the selected line in the commits list, but not of re-rendering the
main view. HandleFocus does that.
When exiting filtering, the HandleFocus call was missing entirely.
The tests needed to be reworked a little bit to make this testable.
At the same time, we change the defaults for both of them to "date" (they were
"recency" and "alphabetical", respectively, before). This is the reason we need
to touch so many integration tests. For some of them I decided to adapt the test
assertions to the changed sort order; for others, I added a SetupConfig step to
set the order back to "recency" so that I don't have to change what the test
does (e.g. how many SelectNextItem() calls are needed to get to a certain
branch).
When toggling the value in the UI we simply overwrite the value in UserConfig;
this would be bad if there was ever a chance that we want to write the user
config back to disk, but it is very unlikely that we can do that, because
currently we have no way to tell which parts of the config come from the global
config file and which ones come from a repo-local one.
The code was copied from StagingController in 0496e3af50, and I did add the new
text in that commit, I just forgot to adapt the code to actually use it.
We only want to do this when the function is called from the remote branches
panel. It can also be called with a selection of local branches in order to
delete their remote branches, but in this case the selection shouldn't be
collapsed because the local branches stay around.
We had code already that was supposed to do this, but it didn't work. It should
have used SetSelection() instead of SetSelectedLineIdx(); the latter doesn't
actually cancel a range selection.
Introduce a new function specifically for collapsing the range after deleting
multiple items, so that clients don't need two calls (we'll add a bunch more in
this branch).
While it's true that the behavior is a little different from the staging panel,
where the staged lines are actually removed from the view and in many cases the
selection stays more or less in the same place, it is still very useful to move
to the next stageable thing in the custom patch building view too.
This improves the experience when selecting a hunk generously with the mouse, by
dragging over it including some context lines above and below. Previously we
would consider the "moving end" of the selection range for whether things need
to be added or removed, but this doesn't make sense if it's a context line. Now
we consider the first actual change line that is included in the range.
CheckMergeOrRebase calls Refresh already. However, it does an async refresh by
default, so we must turn this into a sync refresh so that moving the selection
down by one works even for the very first commit in history. Also, we must add
an explicit call to FocusLine so that the view selection is in sync with the
model selection; previously this was taken care of by the PostRefreshUpdate call
that happens as part of a refresh.
Keep the same commit selected, by moving the selection down by the number of
cherry-picked commits. We also do this when reverting commits, and it is
possible now that we use a sync waiting status.
We also need to turn the refresh that happens as part of CheckMergeOrRebase into
a sync one, so that the commits list is up to date and the new selection isn't
clamped.
For the case of creating a new branch by moving commits to it, we were using the
current (old) branch name in the stash name; change this to use the new name
instead.
Unlike moving a patch to the index, applying or reverting a patch didn't
auto-stash, which means that applying a patch when there's a modified (but
unstaged) file in the working tree would error out with the message "error:
file1: does not match index", regardless of whether those modifications conflict
with the patch or not.
To fix this, we *could* add auto-stashing like we do for the "move patch to
index" command. However, in this case we rather simply stage the affected files
(after asking for confirmation). This has a few advantages:
- it only changes the staging state of those files that are contained in the
patch (whereas auto-stashing always changes all files to unstaged)
- it doesn't unnecessarily show a confirmation if none of the modified files are
affected by the patch
- if the patch conflicts with the modified files, the conflicts were "backwards"
("ours" was the patch, "theirs" the modified file); it is more logical if "ours"
is the current state of the file, and "theirs" is the patch.
It's a little unfortunate that the behavior isn't exactly the same as for "move
patch to index", but for that one we do need the auto-stash because of the
rebase that runs behind the scenes.
Refresh is one of those functions that shouldn't require error handling (similar
to triggering a redraw of the UI, see
https://github.com/jesseduffield/lazygit/issues/3887).
As far as I see, the only reason why Refresh can currently return an error is
that the Then function returns one. The actual refresh errors, e.g. from the git
calls that are made to fetch data, are already logged and swallowed. Most of the
Then functions do only UI stuff such as selecting a list item, and always return
nil; there's only one that can return an error (updating the rebase todo file in
LocalCommitsController.startInteractiveRebaseWithEdit); it's not a critical
error if this fails, it is only used for setting rebase todo items to "edit"
when you start an interactive rebase by pressing 'e' on a range selection of
commits. We simply log this error instead of returning it.
This was added after this PR comment:
https://github.com/jesseduffield/lazygit/pull/3276#discussion_r1469077611
> Can we do a refresh after this reset so that the screen shows that the patch
> has been cancelled? That way, if we cancel on the next popup, the screen will
> be in a valid state.
I don't understand what "cancel on the next popup" means; there is no further
popup after this code.
I took the set of enabled checks from revive's recommended configuration [1],
and removed some that I didn't like. There might be other useful checks in
revive that we might want to enable, but this is a nice improvement already.
The bulk of the changes here are removing unnecessary else statements after
returns, but there are a few others too.
[1] https://github.com/mgechev/revive?tab=readme-ov-file#recommended-configuration
The function would return "head/branchname" when there was either a tag or a
remote with the same name.
While fixing this we slightly change the semantic of the function (and of
determineCheckedOutBranchName, which calls it): for a detached head it now
returns an empty string rather than the commit hash. I actually think this is
better.
Pretty basic fix, didn't seem to have any complications.
I only added the refs/ prefix to the FullRefName() method
to align with other similar methods, and to make this change
not impact any user facing modals.
Fixes: https://github.com/jesseduffield/lazygit/issues/4634
Also adds a test demonstrating that the stash show behavior is now fixed
Previously we would call pullFiles() from the pick() handler if we were not in a
rebase, assuming that the default keybinding for both is "p". This needn't be
the case of course, if the user has remapped one or the other.
The consequence of this was that swapping the keybindings for "pullFiles" and
"pushFiles" would work in all panels except the Commits panel (unless "pick" was
also remapped in the same way).
Fix this by using the new AllowFurtherDispatching mechanism of DisabledReasons
to pass the keybinding on to the next handler.
Moving the getter of the suggested branch name to a separate function
allows us to reuse it in situations where we are not calling the regular
create new branch function, such as move commits to a new branch
Signed-off-by: Elias Assaf <elyas51000@gmail.com>
When refreshing the branches list, we have code to keep the same branch selected
even when the refresh changes the sort order; this code remembers the selected
branch before the refresh, and then tries to select it again afterwards (looking
it up by name) if it is still there.
However, we stored the previously selected branch too early, before even
obtaining the branches list; if the user moved the selection between that point
and the end of the refresh, it would jump back. Fix this by remembering the
previous selection only at the last moment, right before assigning the new
branches slice.
We still have a race condition here between the UI code that manages the
selection as the user presses arrow keys, and the background thread doing the
refresh that reads and restores the selection; however, the race was there
before, and we make it neither better nor worse with this PR. It doesn't seem to
be a problem in practice.
The click handler of MainViewController was registered as a global handler, so
it was used when a side panel was focused that doesn't have a
SwitchToFocusedMainViewController attached (e.g. Status, Worktrees, or
Submodules). This handler would then push the main view context, but with the
code that is meant only for toggling between the main view pair contexts, i.e.
with taking over the parentContext from the otherContext, which doesn't have one
at that point. This would later lead to a crash in onClick because the
parentContext was nil.
Fix this by splitting the click handler in two, one for when it already has the
focus, and one for toggling from the other view, and make these focus specific.
I'm not aware of any real scenario where this can happen, but we have seen one
stack trace where it crashed with an out-of-bounds error in the range expression
below, so there must be a way. And it seems better to guard against it anyway,
rather than assuming it can't happen.
This now allows for leaving the status panel and returning back to the
same log command. Previously any return to the status panel would result
in the next command in the list being shown. Now, you need to press `a`,
with a log command being rendered, to rotate to the next
allBranchesLogCmd.
In one case it was actually called *before* making a commit (when switching from
the commit message panel to committing in the editor). And clearing the
preserved message is the only thing it does, so name it after what it does
rather than when it's called.
Previously it would only clear the message if the panel had been opened with
preserveMessage=true; we don't need this check, because callers know how they
opened the panel, and whether they want to clear the message.
This is no change in behavior because OnCommitSuccess only clears the message
when the commit message panel was opened with preserveMessage=true, which it
isn't in the case of creating a tag.
And this is in fact the desired behavior, because we don't want creating a tag
to interfere with preserving commit messages in any way.
The default binding for ConfirmInEditor is <a-enter>, which has two problems:
- some terminal emulators don't support it, including the default terminal on
Mac (Terminal.app)
- on Windows it is bound to toggling full-screen
Ideally we would use <c-enter> instead (and Command-Enter on Mac), but neither
is possible without https://github.com/gdamore/tcell/issues/671, so for the time
being add an alternate keybinding which works everywhere.
Show both bindings in the footer of the commit description panel if they are
both non-null. While we're at it, fix the footer for the case where either or
both of the keybindings are set to <disabled>.
And finally, change "commit" to "submit" in that footer; we use the same panel
also for creating tags, in which case "commit" is not quite right.
It's never called, the binding ListController.HandleGotoBottom wins.
The functionality of loading more commits is implemented by GetOnFocus, and this
way it works not only for '>', but also for other navigation keys like page
down.
This in itself is not an improvement, because hashes are unique (they are shared
between real commits and rebase todos, but there are so few of those that it
doesn't matter). However, it becomes an improvement once we also store parent
hashes in the same pool; but the real motivation for this change is to also
reuse the hash pointers in Pipe objects later in the branch. This will be a big
win because in a merge-heavy git repo there are many more Pipe instances than
commits.
This makes it easier to copy diff hunks and paste them into code. We only strip
the prefixes if the copied lines are either all '+' or all '-' (possibly
including context lines), otherwise we keep them. We also keep them when parts
of a hunk header is included in the selection; this is useful for copying a diff
hunk and pasting it into a github comment, for example.
A not-quite-correct edge case is when you select the '--- a/file.txt' line of a
diff header on its own; in this case we copy it as '-- a/file.txt' (same for the
'+++' line). This is probably uncommon enough that it's not worth fixing (it's
not trivial to fix because we don't know that we're in a header).
This is very similar to what we are doing for staging or discarding hunks in the
Files panel. Git doesn't allow applying patches with a zero context size (unless
you use the --unidiff-zero option, which is discouraged).
The long story: I want to call this function from RefsHelper; however, I can't
make WorkingTreeHelper a field of RefsHelper because RefsHelper is already a
field in WorkingTreeHelper, so that would be a circular dependency.
The shorter story: there's really little reason to have to instantiate a helper
object in order to call a simple function like this. Long term I would like to
get to a state where a lot more of these helper functions are free-standing, and
you pass in the data they need.
While at it, simplify the implementation of AnyStagedFiles and AnyTrackedFiles
to one-liners.
It's the same, really, except that GetCheckedOutRef() does a check if any
branches exist and returns nil if not. Since we are accessing the returned
branch unconditionally without checking for nil, it seems this check is not
needed here. (The functions we are touching here are called from handlers that
are guarded with itemSelected or singleItemSelected, so we know that at least
one branch exists.)
The goal is to get rid of the dependency to refsHelper.
In this commit this is only possible by pressing '0' in a side panel; we'll add
mouse clicking later in the branch.
Also, you can't really do anything in the focused view except press escape to
leave it again. We'll add some more functionality in a following commit.
Previously we would render the diff for a directory to the main/secondary pair,
but a diff for a file to the staging/stagingSecondary pair. (And similar for
commit files: main/secondary for directories, but
patchBuilding/patchBuildingSecondary for files.)
I always found this confusing and couldn't really understand why we are doing
this; but now it gets in my way because I want to attach a controller to
main/secondary so that they can be focused. So change it to always use the main
context pair for everything we render from a side panel.
We do this because
- it's closer to what you would do on the command line
- it simplifies the code a bit
- it will allow us to support cherry-picking merge commits.
We treat the .git/sequencer/todo file as read-only. Technically it seems it
would be possible to treat it as modifiable in the same way as
.git/rebase-merge/git-rebase-todo, effectively turning a cherry-pick or revert
that stops at a conflict into an interactive rebase; however, git itself doesn't
allow this (there is no "git cherry-pick --edit-todo"), so it seems safer not to
rely on it.
Theoretically it would be possible to allow modifying the rebase todos when a
cherry-pick or revert conflicts in the middle of a rebase. However, it would
introduce a bit of complexity to support this, as we would have to be able to
distinguish between rebasing todos and cherry-picking/reverting todos, which we
currently can't; it could also be a bit error-prone as far as edge cases are
concerned. And it's really a pretty uncommon situation, so it doesn't seem worth
it, and we just forbid all modifications to todos whenever we are cherry-picking
or reverting.
It is useful to see if the conflicted commit was a "pick" or an "edit". What's
more, we're about to add support for showing cherry-picks and reverts, and
seeing that a conflicted commit was a revert is important because its diff is
backwards compared to the diff of the conflicting files in the Files panel.
This is equivalent in the current state of the code, but it will no longer be
after the next commit, because we will introduce a new status value
StatusConflicted. And in a later PR we might add yet another value
StatusCherryPicking to distinguish rebase todos from cherry-pick todos; using
commit.IsTODO is a safer way to check whether a commit is any of these.
When you are in the middle of a rebase, and you cherry-pick a commit which
conflicts, it helps to be clear on whether you are prompted to continue the
cherry-pick or the rebase.
It looks like enums.go was supposed to be file that collects a bunch of enums,
but actually there's only one in there, and since it has methods, it deserves to
be in a file of its own, named after the type.
- Remove REBASE_MODE_NORMAL. It is not the "normal" mode anyway, rather a legacy
mode; we have removed support for it in eb0f7e3d02, so there's no point in
representing it in the enum.
- Remove distinction between REBASE_MODE_REBASING and REBASE_MODE_INTERACTIVE;
these are the same now.
- Rename StatusCommands.IsInInteractiveRebase to IsInRebase.
- Remove StatusCommands.RebaseMode; use StatusCommands.IsInRebase instead.
It is shown either when committing with `w`, or when typing the skipHooks prefix
if there is one. This should hopefully make it clearer when the hooks are run
and when they are not.
We removed prefilling the skipHook prefix in b102646b20 with the intention of
making it clearer that using the prefix in normal commits and typing `w` to skip
hooks are now two independent features.
It turns out that some people liked it with prefilling the prefix and perceive
it as a regression, so put it back in.
But only if we don't have a preserved message; this is an important use case,
when you try to make a normal commit, the hook fails, and then you want to make
the same commit with skipping the hook, but with the same message that you
already typed.
This makes it possible to use date and time in initial values like this:
```yaml
initialValue: 'ruudk/{{ runCommand "date +\"%Y/%-m\"" }}/'
```
I want to use this to configure my BranchPrefix like this:
```yaml
git:
branchPrefix: 'ruudk/{{ runCommand "date +\"%Y/%-m\"" }}/'
```
For the less common conflict types DD, AU, UA, DU, and UD, we would previously
only show "* Unmerged path" in the main view, which isn't helpful. Also, for
some of these we would split the main view and show this text both in the
unstaged changes and staged changes views, which is a bit embarrassing.
Improve this by offering more explanation about what's going on, and what the
most likely way to resolve the situation is for each case.
I can only guess here: maybe they were added to more clearly document the public
interface of the classes? If so, I don't think that works. Developers who are
not familiar with the convention will just add a new public method to the class
without updating the interface.
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.
The current rules for discarding submodule changes is that no other changed item
must be also selected. There are some bugs with the current implementation when
submodules are in folders.
For example, selecting and discarding a folder with only a nested submodule
change will currently do nothing. The submodule changes should be discarded. The
folder only contains submodule changes so it should be no different than
pressing discard on the submodule entry itself.
Also, I noticed range selecting both the folder and the submodule and then
pressing discard would be incorrectly disallowed.
Add verify flag
Add and update integration tests
Rename verify to forceSkipHooks
Adapt CommitSkipHooks integration test to actually use a hook
Remove forceSkipHooks param from OnConfirm et al
Simplify tests
This is useful for copying the entire content of the selected file as it was at
the selected commit.
We only add it to the commit files panel; it is not needed in the files panel,
because there you can simply press "e" to edit the file.
Before we changed the commitPrefix config to a list in #4261, we had this bug
where the defaults section in Config.md would erroneously list the default for
commitPrefix as
git:
commitPrefix:
pattern: ""
replace: ""
This was not correct, commitPrefix was a pointer, and the default for that was
nil, which is not the same.
Now, some people copied and pasted the entire defaults section into their config
files, setting the commitPrefix to an empty pattern (which is not the same as
not setting it at all). And this caused the branch name to be filled in to the
subject field for every commit; see for example
https://github.com/jesseduffield/lazygit/discussions/3632.
New users copying the defaults section into their config file in the current
version no longer have this problem because now that commitPrefix is a list, it
is no longer included in the defaults section. However, the migration that we
added in #4261 would happily carry over those empty strings into a list entry,
so people migrating from an older version still have the broken config in their
config files.
To work around the issue, ignore commit prefix settings whose pattern is an
empty string. I can't imagine a valid reason why people would actually want to
set the pattern to an empty string, so I assume this only comes from the broken
defaults problem described above.
Rewording a commit at the beginning of a long branch can take very long;
without this change, the commit message panel would stay visible with a blinking
cursor during that time, which is very confusing.
This has the slight downside that it will say "Rebasing" in the lower right
corner until the operation is done; but we already have this problem when doing
custom patch operations, or dropping changes from a commit, so it's not new, and
we can think about how to fix all these another time.
WithGpgHandling already does an async refresh when done, so there's no need to
do one here for the case of amending the head commit. On top of that,
WithGpgHandling uses WithWaitingStatus and works in the background, so the
Refresh here would come too early anyway.
All this does is clear the preserved commit message; however, we open the commit
message panel with PreserveMessage: false when rewording, so this is not
necessary.
In ff4ae4a544 we changed the order of the calls to render before selecting the
branch. This was done only to save an extra call to ReApplyFilter, which is done
by refreshView; I claimed that the order of refreshView vs. SetSelectedLineIdx
doesn't matter here. I guess I was wrong about that, it makes the integration
test custom_commands/suggestions_preset.go flaky. To fix this, put the
refreshView call back to where it was (after the SetSelectedLineIdx call), and
instead insert an extra call to ReApplyFilter where necessary to fix the bug
that ff4ae4a544 was trying to fix.
This is very similar to the same menu in the Files panel, except that it works
on whatever diff is currently shown in the main view, including range diffs
either in diffing mode (shift-W), or from a range selection of commits.
When pasting a multi-line commit message into the subject field of the commit
editor, we would interpret the first newline as the confirmation for closing the
editor, and then all remaining characters as whatever command they are bound to,
resulting in executing all sorts of arbitrary commands.
Now we recognize this being a paste, and interpret the first newline as moving
to the description.
Also, prevent tabs in the pasted content from switching to the respective other
panel; simply insert four spaces instead, which should be good enough for the
leading indentation in pasted code snippets, for example.
Jesse's comment from https://github.com/jesseduffield/lazygit/issues/4237:
We recently added a new option to check out a commit's branch from within the
commits, reflog, and sub-commits panels:
https://github.com/user-attachments/assets/0a5cf3f2-6803-4709-ae5a-e4addc061012
After using it for some time, I find it annoying that the default option has
changed. I rarely find myself wanting to check out a branch from the commits
panel, and it's rarer still to want to check out a branch from the reflog and
sub-commits panel. Although there may be use cases for this, it is jarring that
something you can always do (checkout the commit) is harder to do than something
that you can sometimes do (checkout the branch).
We've also had a user complain (see
https://github.com/jesseduffield/lazygit/pull/4117) about their muscle-memory
being broken by the recent change, and I have also fallen victim to this. I
don't think that the new branch checkout option is sufficiently useful to
dislodge the existing keybinding, so let's swap them.
The code that tries to reselect the same branch again uses GetItems, which in
case of filtering is the filtered list. After replacing the branches slice with
a new one, the filtered list is no longer up to date, so we must reapply the
filter before working with it. It so happens that refreshView does that, so
simply call that before setting the selection again; I don't think the order
matters in this case. Otherwise we'd have to insert another call to
ReApplyFilter before the call to GetItems, which we can avoid this way.
Note that this doesn't actually make anything work better in the case of
deleting a branch, since we can't reselect the deleted branch anyway of course.
But it avoids a possible crash if the branch that was deleted was the last one
in the unfiltered list.
This includes the "only conflicting" status that the user can't switch to
themselves. We display it anyway to give a hint that files are being filtered,
and to let them know that they can turn the filter off if they want to.
I renamed the "Reset filter" item to "No filter" to make it look more like a
state than an action, so that it fits the radio button concept better.
When there are conflicts and we set the filter to show only conflicting files,
then none of the radio buttons light up, which is slightly strange. I guess it's
ok though.
We don't need to maintain additional state to allow this; all we need to do is
take over the filter only when the number of conflicting files goes from zero to
non-zero, rather than every time it is non-zero.
The only problem is that we don't allow users to go back to showing only
conflicted files, but that's just because we don't have that as an entry in the
menu. And I don't think it's a problem.
This handles the situation where the user's own config says to not show
untracked files, as is often the case with bare repos managing a user's
dotfiles.
Git diff and patch doesn't work reliably with a context size of 0, so disable it
in this case (and discarding changes as well). Magit does the same, see
https://github.com/magit/magit/issues/4222.
Staging entire files by pressing space in the Files panel is still possible, of
course.
When pushing a branch that didn't have an upstream yet, we use the command line
git push --set-upstream origin HEAD:branch-name
The HEAD: part of this is too unspecific; when checking out a different branch
while the push is still running, then git will set the upstream branch on the
newly checked out branch, not the branch that was being pushed. This might be
considered a bug in git; you might expect that it resolves HEAD at the beginning
of the operation, and uses the result at the end.
But we can easily work around this by explicitly supplying the real branch name
instead of HEAD.
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.
There are two ways to jump to the editor on a specific line: pressing `e` in the
staging or patch building panels, or clicking on a hyperlink in a delta diff. In
both cases, this works perfectly in the unstaged changes view, but in other
views (either staged changes, or an older commit) it can often jump to the wrong
line; this happens when there are further changes to the file being viewed in
later commits or in unstaged changes.
This commit fixes this so that you end up on the right line in these cases.
When the user checks out a commit which has a local branch ref attached
to it, they can select between checking out the branch or checking out
the commit as detached head.
Sometimes we populate the commit message panel with a pre-created commit
message. The two cases where this happens is:
- you type `w` to commit, in which case we put the skipHookPrefix in the subject
- you have a commitPrefix pattern, in which case we match it against the branch
name and populate the subject with the replacement string if it matches
In either case, if you have a preserved commit message, we use that.
Now, when you use either of these and then cancel, we preserve that initial,
unchanged message and reuse it the next time you commit. This has two problems:
it strips spaces, which is a problem for the commitPrefix patterns, which often
end with a space. And also, when you change your config to experiment with
commitPrefix patterns, the change seemingly doesn't take effect, which can be
very confusing.
To fix both of these problems, only preserve the commit message when it is not
identical to the initial message.
So far, lines in the view corresponded 1:1 to lines in the patch. Once we turn
on wrapping for the staging view (which we don't do yet), this is no longer
true, so we need to convert from view lines to patch lines or vice versa all
over the place.
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.
This is also what we do in the staging controller, and it makes it so that when
you exit the patch building view and then enter it again (for another file, or
the same one) we select the first hunk again.
For non-merge commits we change "pick" to "drop" when we delete them. We do this
so that we can use the same code for dropping a commit no matter whether we are
in an interactive rebase or not. (If we aren't, we could just as well delete the
pick line from the todo list instead of setting it to "drop", but if we are, it
is better to keep the line around so that the user can change it back to "pick"
if they change their mind.)
However, merge commits can't be changed to "drop", so we have to delete them
from the todo file. We add a new daemon instruction that does this.
We still don't allow deleting a merge commit from within an interactive rebase.
The reason is that we don't show the "label" and "reset" todos in lazygit, so
deleting a merge commit would leave the commits from the branch that is being
merged in the list as "pick" commits, with no indication that they are going to
be dropped because they are on a different branch, and the merge commit that
would have brought them in is gone. This could be very confusing.
Not much of a change in behavior, because moving merge commits was already not
possible. However, it failed with a cryptic error message ("Todo fa1afe1 not
found in git-rebase-todo"), so disable it properly instead.
It is needed by both call sites of this function. This has the added benefit
that the argument doesn't unnecessarily show up in the status view when diffing
mode is on.
In 67b8ef449c we changed the "edit" command to insert a "break" after the
selected commit, rather than setting the selected todo to "edit". The reason for
doing this was that it now works for merge commits too.
Back then, I claimed "In most cases the behavior is exactly the same as before."
Unfortunately that's not true, there are two reasons why the previous behavior
was better (both are demonstrated by tests earlier in this branch):
- when editing the last commit of a branch in the middle of a stack of branches,
we are now missing the update-ref todo after it, which means that amending the
commit breaks the stack
- it breaks auto-amending (see the added test earlier in this branch for an
explanation)
For these reasons, we are going back to the previous approach of setting the
selected commit to "edit" whenever possible, i.e. unless it's a merge commit.
The only scenario where this could still be a problem is when you have a stack
of branches, and the last commit of one of the branches in the stack is a merge
commit, and you try to edit that. In my experience with stacked branches this is
very unlikely, in almost all cases my stacked branches are linear.
We allow deleting remote branches (or local and remote branches) only if *all*
selected branches have one.
We show the a warning about force-deleting as soon as at least one of the
selected branches is not fully merged.
The added test only tests a few of the most interesting cases; I didn't try to
cover the whole space of possible combinations, that would have been too much.
Since we want to select multiselections, this will make it easier to pass a
slice of remote branches. It does require that for the case of the local
branches panel we need to synthesize a RemoteBranch object from the selected
local branch, but that's not hard.
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.
When creating a PR against a selected branch (via O = "create pull request
options"), the user will first be asked to select a remote (if there is more
than one). After that, the suggestion area is populated with all remote branches
at that origin - instead of all local ones. After all, creating a PR against a
branch that doesn't exist on the remote won't work.
Please note that for the "PR is not filed against 'origin' remote" use case
(e.g. when contributing via a fork that is 'origin' to a GitHub project that is
'upstream'), the opened URL will not be correct. This is not a regression and
will be fixed in an upcoming PR.
Fixes#1826.
As far as I understand, this was needed back when the staging context was still
responsible for rendering its highlight (as opposed to the gocui view, as it is
today). It was necessary to call it with isFocused=false so that it removed the
highlight. The isFocused bool is no longer used today (and we'll remove it in
the next commit), so there's no need to render the view here any more.
This fixes flickering when leaving the staging view by pressing escape. The
reason is that in this case the patch state was already set to nil by the time
we get here, so we would render an empty view for a brief moment.
On top of that, it fixes unwanted scrolling to the top when leaving the staging
view. The reason for this is that we have code in layout that scrolls views up
if needed, e.g. because the window got taller or the view content got shorter
(added in #3839). This kicked in because we emptied the view, and scrolled it
all the way to the top, which we don't want.
After pasting commits once, we hide the cherry-picking status (as if it had been
reset), and no longer paint the copied commits with blue hashes; however, we
still allow pasting them again. This can be useful e.g. to backport a bugfix to
multiple major version release branches.
Currently we try to delete a branch normally, and if git returns an error and
its output contains the text "branch -D", then we prompt the user to force
delete, and try again using -D. Besides just being ugly, this has the
disadvantage that git's logic to decide whether a branch is merged is not very
good; it only considers a branch merged if it is either reachable from the
current head, or from its own upstream. In many cases I want to delete a branch
that has been merged to master, but I don't have master checked out, so the
current branch is really irrelevant, and it should rather (or in addition) check
whether the branch is reachable from one of the main branches. The problem is
that git doesn't know what those are.
But lazygit does, so make the check on our side, prompt the user if necessary,
and always use -D. This is both cleaner, and works better.
See this mailing list discussion for more:
https://lore.kernel.org/git/bf6308ce-3914-4b85-a04b-4a9716bac538@haller-berlin.de/
This might seem controversial; in many cases the client code gets longer,
because it needs an extra line for an explicit `return nil`. I still prefer
this, because it makes it clearer which calls can return errors.
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.
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.
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.
It doesn't seem to be used by anything, it looks like we only need to implement
the method so that the IController interface is satisfied. But still, it doesn't
hurt to be correct here, and might avoid confusion in the future when somebody
does try to use the method.
It still doesn't update the view immediately when reloading the config, only the
next time the status panel is focused. But at least it does it then; writing the
code to update the panel when reloading the config is probably not justifiable.
The comments that I'm deleting here explain why we need the bool; however, in
our case that's a theoretical issue. It would only arise if we ever were to pass
a nil context to SetParentContext, which we never do.
Also, use the user's shell (from the SHELL env variable) instead of bash. Both
of these together allow users to use their shell aliases or shell functions in
the interactive command prompt.
The current behaviour when creating a new branch off of a remote branch
is to always track the branch it was created from.
For example, if a branch 'my_branch' is created off of the remote branch
'fix_crash_13', then 'my_branch' will be tracking the remote
'fix_crash_13' branch.
It is common practice to have both the local and remote branches named
the same when the local is tracking the remote one. Therefore, it is
reasonable to expect that 'my_branch' should not track the remote
'fix_crash_13' branch.
The new behaviour when creating a new branch off of a remote branch is
to track the branch it was created from only if the branch names match.
If the branch names DO NOT match then the newly created branch will not
track the remote branch it was created from.
For example, if a user creates a new branch 'fix_crash_13' off of the
remote branch 'fix_crash_13', then the local 'fix_crash_13' branch will
track the remote 'fix_crash_13' branch.
However, if the user creates a new branch called 'other_branch_name' off
of the remote branch 'fix_crash_13', then the local 'other_branch_name'
branch will NOT track the remote 'fix_crash_13' branch.
The default shortcut to open git difftool (ctrl+t) is not available on
the "Local Branches" window. It is available when selecting a commit
from a local branch, a remote branch, or a tag from the "Local Branches"
window.
This is inconsistent since branches or tags are also commits, the
shortcut should also work on them directly.
This commit remedies this inconsistency by allowing the use of the
shortcut directly on a branch or a tag. The shortcut works both in the
"standard" mode and the "diffing" mode.
The highlight is normally turned off in HandleFocusLost, but that's not called
when using ReplaceContext (and changing this would be a lot of work, it seems),
so turn it off manually here for now.
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.
This is how we do it for confirmation with suggestions too, so be consistent. It
will make things easier later in this branch if we only have one context per
"panel" on the stack, even if the panel consists of two views.
Concretely this means:
- only push the message context onto the stack when opening the panel (this
requires making the description view visible manually; we do the same for
suggestions)
- when switching between message and description, use ReplaceContext rather than
PushContext
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).
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.
