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.
It can optionally be used to set the title of the panel that shows the output of
a command (when showOutput is true). If left unset, the command string is used
as the title.
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 a triangular workflow the branch that you're pulling from is not the same as
the one that you are pushing to. For example, some people find it useful to set
the upstream branch to origin/master so that pulling effectively rebases onto
master, and set the push.default git config to "current" so that "feature"
pushes to origin/feature.
Another example is a fork-based workflow where "feature" has upstream set to
upstream/main, and the repo has remote.pushDefault set to "origin", so pushing
on "feature" pushes to origin/feature.
This commit adds new fields to models.Branch that store the ahead/behind
information against the push branch; for the "normal" workflow where you pull
and push from/to the upstream branch, AheadForPush/BehindForPush will be the
same as AheadForPull/BehindForPull.
This guards against accidentally renaming a model field and thereby breaking
user's custom commands. With this change we'll get a build failure when we do
that.
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.
Deadlock reporting broke in e1ceb6892a; since then, it was *off* when running
debug builds normally, but *on* when debugging an integration test. Both of
which are exactly opposite of what we want.
Change `func displayCommit()` so all the individual strings are built first,
then the whole thing `cols` is put together. Before, most strings were built
prior to constructing `cols`, but a few were built inside the `cols`
construction.
Sometimes it takes a while to get PRs accepted upstream, and this blocks our
progress. Since I'm pretty much the only one making changes there anyway, it
makes sense to point to my fork directly.
It is now only used as the error handler that is passed to gocui.Gui on
construction; it's not a client-facing API any more. Also, it doesn't have to
handle gocui.ErrQuit, as gocui takes care of that.
This lets us get rid of a few more calls to Error(), and it simplifies things
for clients of OnWorker: they can simply return an error from their callback
like we do everywhere else.
In Gui.onWorker we only make the minimum possible change to get things to
compile after the API-breaking change of the gocui update; we'll make this
cleaner later in this branch.
We upgraded our minimum Go version to 1.21 in commit
57ac9c2189. We can now replace our
`utils.Min` and `utils.Max` functions with the built-in `min` and `max`.
Reference: https://go.dev/ref/spec#Min_and_max
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Make it recognize URLs wrapped in angle brackets, and followed by punktuation.
We don't need this for the status panel, but we will need it for confirmation
panels.
Changing globals in the init() function of a test file is a bad idea, as it
affects all other tests that run after it. Do it explicitly in each test
function that needs it, and take care of restoring the previous value
afterwards.
When exiting filtering mode, we currently keep the selection index the same in
the commits panel. This doesn't make sense at all, since the index in the
filtered view has no relation to the index in the unfiltered view.
I often use filtering mode (either by path or by author) to find a given commit
faster than I would otherwise be able to. When exiting filtering mode, it's
useful to keep the same commit selected, so that I can look at the surrounding
commits, see which branch it was a part of, etc. So reselect the commit again
after exiting filtering mode.
Sometimes this is not possible, most likely when the commit is so long ago that
it's outside of the initial 300 range. In that case, at least select the commit
again that was selected before I entered filtering; this is still better than
arbitrarily keeping the same selection index.
It would crash when some keybindings are set to null, and the filter string is
such that only those keybindings remain visible.
The reason for the crash is that when inserting non-model items (menu section
headers in this case) you specify a column to align them to. This works on the
assumption that the number of columns is always the same. It can cope with the
case that columns are removed because they are empty for all items; but it can't
cope with the case that the getDisplayStrings function returns a lower number of
columns.
And this is what happened here: MenuViewModel.GetDisplayStrings would omit the
keybinding column when none of the entries have a keybinding. This logic is
unnecessary, the generic list rendering mechanism takes care of this, so
removing this logic fixes the crash.
We do have to make sure though that the column is really empty when there's no
keybinding, so change the logic to use FgCyan only when there's a keybinding.
I often copy hashes in the commits panel in order to paste them into Github
comments (or other places), and I can't stand it when they have the full length.
I picked a default of 12 for this; I find this to be a good middle ground
between being reliable in large repos (12 still works in the linux kernel repo
today, but it might not be enough in really huge repos) and not being too ugly
(many smaller repos can probably get away with less).
We deliberately don't change this for the "Copy to clipboard" menu, since this
gives users a way to copy the unabbreviated sha if they need this occasionally.
To support this, we turn the confirmation prompt of the "Create fixup commit"
command into a menu; creating a fixup commit is the first entry, so that
"shift-F, enter" behaves the same as before. But there are additional entries
for creating "amend!" commits, either with or without file changes. These make
it easy to reword commit messages of existing commits.
We pass all of them to a single editor command, hoping that the editor will be
able to handle multiple files (VS Code and vim do).
We ignore directories that happen to be in the selection range; this makes it
easier to edit multiple files in different folders in tree view. We show an
error if only directories are selected, though.
We've seen a lot of issues recently where people complain that lazygit doesn't
behave as documented, but that was only because they were running the latest
release but were looking at the documentation of master. Make the documentation
links in the status panel point to the release that they are using in the hope
that this will help a little bit with this problem.
Some operations don't support switching to the editor from the commit message
panel; an example is the commit message panel that appears when moving a custom
patch into a new commit. Disable the "open in editor" menu entry in this case,
instead of silently doing nothing.
Previously we would hide it if no onSwitchToEditor function was set; that was
from a time when <c-o> was bound directly to the switch-to-editor command. Now
it is bound to showing a menu, and that menu is always available even if no
onSwitchToEditor function is set. (We rather need to disable the switch to
editor item _within_ that menu, see next commit.)
For die-hard fuzzy-searching fans it's probably in the way, so taking it out
makes fuzzy filtering work better. For substring filtering it always retains the
sort order anyway.
By default we now search for substrings; you can search for multiple substrings
by separating them with spaces. Add a config option gui.filterMode that can be
set to 'fuzzy' to switch back to the previous behavior.
Lazygit has two ways to decide whether it needs to ask the user to force-push:
1. if it knows ahead of time that the push will fail because the branch has
diverged, by looking at the incoming/outgoing information that it shows as ↑3↓7.
2. by examining the error that comes back when the push has failed.
The second situation should happen only rarely, because lazygit fetches every
minute by default, so the ↑3↓7 information is usually up to date. It might not
be if the user turned off auto-fetch (or increased the auto-fetch interval).
However, in this case it's almost always harmful to prompt the user to
force-push, because we know that the reason for diverging is that something was
pushed to the remote, and we would wipe it out by force-pushing. In such a
situation, the more likely user action is to pull the remote changes and then
push normally again.
So just remove the second prompt, and replace it by a better error message when
we detect that updates were rejected remotely.
A little bit of history archeology reveals that the second prompt was added at a
time where we didn't have the first one yet, so at that time it made sense to
have it; but when the first prompt was added, we should have removed the second.
So far, the only situation where we called SetSelectionRangeAndMode was one
where the range could only get larger (in startInteractiveRebaseWithEdit, in
which case update-ref todos can be inserted by the rebase). However, in the last
commit we introduced a new call site where the range can get smaller, including
being reduced to a single item. Since this is indistinguishable from a single
selection, set the mode to none in this case; without this, hitting escape would
seemingly do nothing because it collapses the empty range selection.
... and when recalling a commit message from an old commit by pressing up-arrow.
This is necessary because committing turns our soft line breaks into real ones,
but when rewording we want to turn them back into soft ones again, so that it's
possible to insert words at the beginning of a paragraph and have everything
rewrap nicely.
This is only a best effort; the algorithm only removes those hard line breaks
that can be removed without changing the way the message looks. This works well
when the previous commit message was wrapped at the same width, which for most
users should be the most common case; but if it wasn't, the result is not great.
Specifically, if the old wrap width was smaller, some hard line breaks just
won't be removed; if it was wider though, you'll get an unpleasant comb effect
with alternating long and short lines. In such a case it's best to switch to the
editor and use whatever wrapping features you have there (e.g. alt-Q).
This should arguably have been done in b133318b40 already; it's becoming more
important now because we're going to extend the common code with more logic in
the next commit.
It starts a rebase on the bottom-most commit of the range, and sets all the
selected commits to "edit" (skipping merge commits, because they can't be
edited).
The additional branch head icon is more confusing than useful in this situation.
The update-ref entries show very clearly where the branch heads will go when
continuing the rebase; the information where the branch heads used to be before
the rebase is not really needed here, and just makes the display more confusing.
I'm not adding more tests here because the changes to the existing tests
demonstrate the change clearly enough.
A common workflow for me is to create a fixup commit from only some of my
current changes; to do that, I enter a file, stage a few hunks, and then want to
invoke ctrl-f to find the base commit for these changes. Currently I need to esc
back to the files panel in order to do that; it's more convenient to be able to
do this right from the staging panel.
This commit introduces a new feature to the commit view, allowing users
to filter commits based on the author's name or email address. Similar
to the existing path filtering functionality, accessible through <c-s>,
this feature allows users to filter the commit history by the currently
selected commit's author if the commit view is focused, or by typing in
the author's name or email address.
This feature adds an entry to the filtering menu, to provide users with
a familiar and intuitive experience
Calling "git reset" on the command line (without further arguments) defaults to
--mixed, which is reason enough to make it the default for us, too.
But I also find myself using --mixed more often than --soft. The main use case
for me is that I made a bunch of WIP commits, and want to turn them into real
commits when I'm done hacking. I select the last commit before the WIP commits
and reset to it, leaving all changes of all those commits in the working
directory. Since I want to start staging things from there, I prefer those
modifications to be unstaged at that point, which is what --mixed does.
For some lists it is useful to keep the same sort order when filtering (rather
than sorting by best match like we usually do). Add an optional function to
FilteredList to make this possible, and use it whenever we show lists of things
sorted by date (branches, stashes, reflog entries), as well as menu items
because this allows us to keep the section headers in the keybindings menu,
which is useful for understanding what you are looking at when filtering.
After #3283 we need to read more lines initially so that the scrollbar goes to
its minimal height of 1 for long diffs. Without this, it would start with a
height of 2 and then become smaller after you scroll down half the window
height.
This fixes two problems:
- each time the custom commands panel was opened, the history of commands would
be shown in reversed order compared to last time. (The reason is that
lo.Reverse modifies the slice in place rather than just returning a new,
reversed slice.)
- when executing a previous command again (either by typing it in again, or by
picking it from the history), it should move to the beginning of the history,
but didn't.
We fix this by storing the history in reversed order (as the user sees it in
the panel), this makes the logic simpler. We just have to prepend rather
than append newly added commands now.
While this is theoretically a breaking change, it's not worth bothering because
the order was wrong for existing users in 50% of the cases anyway.
Scenario:
- show the files of a commit, escape out of it again
- start an interactive rebase of a stack of branches, with the rebase.updateRefs
git config set to true
- select one of the update-ref todos
- trigger a refresh (either manually or by bringing lazygit's terminal window to
the front)
This results in an error message "fatal: ambiguous argument '': unknown revision
or path not in the working tree."
Fix this by putting another band-aid on the check for the commit files refresh.
This is the easiest way to fix the problem, but I don't think it's the best one.
We shouldn't be refreshing the commit files context at all if it isn't visible,
because it's pointless; there's no way to switch to it again except by calling
viewFiles again with a specific ref. But I'm too lazy too figure out how to do
that right now.
After discarding file changes from the commit, the was still referencing
these indexes as being part of the range select. The consequence was
needing to hit escape twice to exit commit files in some situations.
Canceling the range select after discarding changes fixes that.
The waiting status shouldn't happen until after the user has responded
to the popup.
Since we're not giving a standalone prompt about clearing the patch, all
of the business in `discard` doesn't need to be in a function any more
It can be tedious after each cherry-pick opearation to clear the
selection by pressing escape in order for lazygit to stop displaying
info about copied commits. Also, it seems to be a rare case to
cherry-pick commits to more than one destination.
The simplest solution to address this issue is to clear the selection
upon paste.
The only exception is a merge conflict. Initially, I wanted to clear
selected commits in this scenario too. During a discussion we found out
that it may be convenient to have the copied commits still around.
Aborting the rebase and pasting the commits in the middle of a branch
can be a valid use case.
To do that, change the "Apply fixup commits" command to show a menu with the two
choices "in current branch" and "above the selected commit"; we make "in current
branch" the default, as it's the more useful one most of the time, even though
it is a breaking change for those who are used to "shift-S enter" meaning
"squash above selected".
Before this commit, we had pkg/integration/tests/submodule/add.go
failing with a panic. I'm pretty sure the issue is this: we're now
calling quite a few GetDisabledReason calls on each layout() call,
and if a background thread happens to update a model slice while
we're doing this, we can end up with a selection index that's now
out of bounds because it hasn't been clamped to match the new list
length.
Specifically, here we had the selected index being -1 (the list starts
empty and somehow the value is -1 in this case) and then the list
gets a new submodule so the length is now 1, but the list cursor
doesn't know about this so remains on the old value. Then we confirm
the length is greater than zero and try to get the selected submodule
and get an out of bounds error.
This commit fixes the issue by clamping the selected index whenever
we get the length of the list so that it stays in-sync. This is not
a perfect solution because the length can change at any time, but
it seems to reliably fix the test, and using mutexes didn't seem to
make a difference.
Note that we're swapping the order of IFileTree and IListCursor in
the file tree view model to ensure that the list cursor's Len()
method is called (which performs the clamping).
Also, comment from the PR:
This 'trait' pattern we're using is convenient but can lead to awkward
situations. In this case we have both the list view model and the
(embedded) list cursor with a Len() method. The list cursor Len()
method just calls the list view model Len() method. But I wanted
to make it that the list view model now calls ClampSelection() on the
list cursor whenever it obtains the length. This will cause an
infinite loop because ClampSelection() internally calls Len()
(which calls the list view model's Len() method which in turn
calls ClampSelection() again, etc).
The only reason we were passing the list view model into the list
cursor was to supply the length method, so now we're just doing
that directly, and letting the list view model delegate the Len()
call to the list cursor, which now itself calls ClampSelection.
As part of making lazygit more discoverable, there are certain keys which you almost certainly
need to press when you're in a given mode e.g. 'v' to paste commits when cherry-picking. This
commit prominently shows these keybinding suggestions alongside the others in the option view.
I'm using the same colours for these keybindings as is associated with the mode elsewhere e.g.
yellow for rebasing and cyan for cherry-picking. The cherry-picking one is a bit weird because
we also use cyan text to show loaders and app status at the bottom left so it may be confusing,
but I haven't personally found it awkward from having tested it out myself.
Previously we would render these options whenever a new context was activated, but now that we
need to re-render options whenever a mode changes, I'm instead rendering them on each screen
re-render (i.e. in the layout function). Given how cheap it is to render this text, I think
it's fine performance-wise.
This adds a bunch of tooltips to keybindings and updates some keybinding descriptions (i.e. labels).
It's in preparation for displaying more keybindings on-screen (in the bottom right of the screen),
and so due in part to laziness it shortens some descriptions so that we don't need to manage both
a short and long description (for on-screen vs in-menu). Nonetheless I've added a ShortDescription
field for when we do want to have both a short and long description.
You'll notice that some keybindings I deemed unworthy of the options view have longer descriptions,
because I could get away with it.
This is useful if you want to move a range of commits, so you select them, and
then realize it's better to do it in an interactive rebase. Pressing 'i'
preserves the range now.
This helps work around bugs in editors that may get confused about relative
paths (like nvim-remote, see https://github.com/neovim/neovim/issues/18519), and
shouldn't have any negative effect on others.
As part of this, you must now press enter on a merge conflict file
to focus the merge view; you can no longer press space and if you do
it will raise an error.
I prefer this because I almost never use sticky range selections. Also, this
fixes the issue that just clicking a line in a diff (without dragging) already
creates a range selection. It still does, technically, but it's no longer a
problem because a non-sticky one-line range selection behaves the same as a
non-range selection.
Something dumb that we're currently doing is expecting list items
to define an ID method which returns a string. We use that when copying
items to clipboard with ctrl+o and when getting a ref name for diffing.
This commit gets us a little deeper into that hole by explicitly requiring
list items to implement that method so that we can easily use the new
helper functions in list_controller_trait.go.
In future we need to just remove the whole ID thing entirely but I'm too
lazy to do that right now.
Fixes https://github.com/jesseduffield/lazygit/issues/3077
Show unstaged file names in default colour
Previously, we had the following rules:
* file names were in red when unstaged or partially staged
* directory names were in red if unstaged, yellow if partially staged,
and
green if fully staged
Red text on a black background can be hard to read, so instead I'm
changing it
so that unstaged files have their names in the default text colour.
I'm also making it so that partially staged files are in yellow, just
like how
partially staged directories are yellow (same deal with the commit files
view
when adding to a custom patch).
So the new rules are:
* unstaged files/directories use the default colour
* partially staged files/directories are in yellow
* fully staged files/directories are in green
I've also done a refactor on the code clean up some dead code from when
the file tree
outline was drawn with box characters, and I've made it so that the
indentation in
each line is handled inside the function that draws the line rather than
in the recursive
parent function. This makes it easier to experiment with things like
showing the file
status characters on the left edge of the view (admittedly after
experimenting with it,
I decided I didn't like it). Apologies for having a refactor and a
functional change
in the one commit but by the time I was done, I couldn't be bothered
going back and
retroactively splitting it into two halves.
Previously, we had the following rules:
* file names were in red when unstaged or partially staged
* directory names were in red if unstaged, yellow if partially staged, and
green if fully staged
Red text on a black background can be hard to read, so instead I'm changing it
so that unstaged files have their names in the default text colour.
I'm also making it so that partially staged files are in yellow, just like how
partially staged directories are yellow (same deal with the commit files view
when adding to a custom patch).
So the new rules are:
* unstaged files/directories use the default colour
* partially staged files/directories are in yellow
* fully staged files/directories are in green
I've also done a refactor on the code clean up some dead code from when the file tree
outline was drawn with box characters, and I've made it so that the indentation in
each line is handled inside the function that draws the line rather than in the recursive
parent function. This makes it easier to experiment with things like showing the file
status characters on the left edge of the view (admittedly after experimenting with it,
I decided I didn't like it). Apologies for having a refactor and a functional change
in the one commit but by the time I was done, I couldn't be bothered going back and
retroactively splitting it into two halves.
This wasn't necessary before, because the only available branch sorting option
was by recency, so the sort order couldn't change except by checking out
branches. Now, you can sort by committer date, so the branch order can change by
fetching; in this case it's important to keep the same branch selected. One
important use case is to rebase the checked-out branch onto master; you select
master, press "f" to fetch it (this can now change its position in the list),
and then press "r" to rebase. To make this work smoothly it's important to keep
master selected after pressing "f".
Often if a test fails and there's an unaknowledged toast message, that message will
explain why the test failed. Given that we don't display toast messages in
integration tests when they run (for reasons I can't recall right now), we need to
log it as part of the error message.
We don't need it there so no need to enable it.
I'm leaving the disabled reason checks there, even though they're now redundant,
because they're only one-liners and they communicate intent.
We want to show an error when the user tries to invoke an action that expects only
a single item to be selected.
We're using the GetDisabledReason field to enforce this (as well as DisabledReason
on menu items).
I've created a ListControllerTrait to store some shared convenience functions for this.
This requires us to change the 'v' keybinding for paste to something else,
now that 'v' is used globally for toggling range select. So I'm using
'shift+v' and I'm likewise changing 'c' to 'shift+c' for copying, so
that they're consistent.
We will need to clearly communicate this change in keybindings.
The only time we should call SetSelectedLineIdx is when we are happy for a
select range to be retained which means things like moving the selected line
index to top top/bottom or up/down a page as the user navigates.
But in every other case we should now call SetSelection because that will
set the selected index and cancel the range which is almost always what we
want.
This is the highest priority of the escape actions because it's the thing you're
most likely to want to do upon hitting escape if you have a range selected.
Applying this to the staging/patch-building views is tricky: if we want this logic
for when a range of lines is selected, we'll also need to apply it when a hunk
is selected too. I still think it's worth it though: I've often accidentally
escaped from the staging view when trying to cancel a range selection.
We're not fully standardising here: different contexts can store their range state however
they like. What we are standardising on is that now the view is always responsible for
highlighting the selected lines, meaning the context/controller needs to tell the view
where the range start is.
Two convenient benefits from this change:
1) we no longer need bespoke code in integration tests for asserting on selected lines because
we can just ask the view
2) line selection in staging/patch-building/merge-conflicts views now look the same as in
list views i.e. the highlight applies to the whole line (including trailing space)
I also noticed a bug with merge conflicts not rendering the selection on focus though I suspect
it wasn't a bug with any real consequences when the view wasn't displaying the selection.
I'm going to scrap the selectedRangeBgColor config and just let it use the single line
background color. Hopefully nobody cares, but there's really no need for an extra config.
This adds range select ability in two ways:
1) Sticky: like what we already have with the staging view i.e. press v then use arrow keys
2) Non-sticky: where you just use shift+up/down to expand the range
The state machine works like this:
(no range, press 'v') -> sticky range
(no range, press arrow) -> no range
(no range, press shift+arrow) -> nonsticky range
(sticky range, press 'v') -> no range
(sticky range, press arrow) -> sticky range
(sticky range, press shift+arrow) -> nonsticky range
(nonsticky range, press 'v') -> no range
(nonsticky range, press arrow) -> no range
(nonsticky range, press shift+arrow) -> nonsticky range
Previously we included all navigation keybindings from all views in the keybindings menu, meaning
if you pressed enter on 'next page' in the commits view, you'd end up triggering the action
in the sub-commits view.
Because we obtain disabled reasons after every action, we need to keep the code for doing so
super fast. As such, we should not be hitting the filesystem to get rebase state, instead
we should just get the cached state.
I feel like we should actually be using the cached state everywhere like we do with all
our other models if only for the sake of consistency.
A common issue I have is that I want to move a commit from the top of my branch
all the way down to the first commit on the branch. To do that, I need to navigate
down to the first commit on my branch, press 'e' to start an interactive rebase,
then navigate back up to the top of the branch, then move my commit back down to
the base. This is annoying.
Similarly annoying is moving the commit one-by-one without explicitly starting
an interactive rebase, because then each individual step is its own rebase which
takes a while in aggregate.
This PR allows you to press 'i' from the commits view to start an interactive
rebase from an 'appropriate' base. By appropriate, we mean that we want to start
from the HEAD and stop when we reach the first merge commit or commit on the main
branch. This may end up including more commits than you need, but it doesn't make
a difference.
The algorithm works by blaming the deleted lines, so if a hunk contains only
added lines, we can only hope that it also belongs in the same commit. Warn the
user about this.
Note: the warning might be overly agressive, we'll have to see if this is
annoying. The reason is that it depends on the diff context size whether added
lines go into their own hunk or are grouped together with other added or deleted
lines into one hunk. However, our algorithm uses a diff context size of 0,
because that makes it easiest to parse the diff; this results in hunks having
only added lines more often than what the user sees. For example, moving a line
of code down by two lines will likely result in a single hunk for the user, but
in two hunks for our algorithm. On the other hand, being this strict makes the
warning consistent. We could consider using the user's diff context size in the
algorithm, but then it would depend on the current context size whether the
warning appears, which could be confusing. Plus, it would make the algorithm
quite a bit more complicated.
There are two possible fixes for this bug, and they differ in behavior when
rewording a commit. The one I chose here always splits at the first line feed,
which means that for an improperly formatted commit message such as this one:
This is a very long multi-line subject,
which you shouldn't really use in git.
And this is the body (we call it "description" in lazygit).
we split after the first line instead of after the first paragraph. This is
arguably not what the original author meant, but splitting after the first
paragraph doesn't really work well in lazygit, because we would try to put both
lines into the one-line subject field of the message panel, and you'd only see
the second and not even know that there are more.
The other potential fix would have been to join subject and description with two
line feeds instead of one in JoinCommitMessageAndDescription; this would have
fixed our bug in the same way, but would result in splitting the above message
after the second line instead of the first. I think that's worse, so I decided
for the first fix.
While we're at it, simplify the code a little bit; strings.Cut is documented to
return (s, "") when the separator is not found, so there's no need to do this on
our side.
We do have to trim spaces on the description now, to support the regular reword
case where subject and body are separated by a blank line.
Without this it's not reliably possible to ask whether a given view is visible
by asking
windowHelper.TopViewInWindow(context.GetWindowName()) == context.GetView()
because there could be transient, invisible contexts after it in the Z order.
I guess it's a bit of a coincidence that this has never been a problem so far.
The output of the GetWindowDimensions function is hard to understand just by looking at it,
so I've added a helper function in the tests to render the window layout as text, so that
in order to create a new test you just come up with some args and paste the output as the
expected output.
This has the same downsides that any snapshot-based testing has: it's more brittle than
targeted assertions. But it is much easier to make sense of these snapshots than it is
to make sense of more fine-grained assertions, and I like the fact that these tests can
serve as documentation.
We are also removing the single-character padding on the left/right edges of the bottom
line because it's unnecessary
Unfortunately we need to create views for each spacer: it's not enough to just
layout the existing views with padding inbetween because gocui only renders
views meaning if there is no view in a given position, that position will just
render whatever was there previously (at least that's what I recall from talking
this through with Stefan: I could be way off).
Co-authored-by: Stefan Haller <stefan@haller-berlin.de>
It sounds like at some point we only showed a slash as the search prompt, but I
dug a bit through the history and couldn't find a state of the code where that
was the case. (shrug)
Situations where a view's width changes:
- changing screen modes
- enter staging or patch building
- resizing the terminal window
For the first of these we currently have special code to force a rerender, since
some views render different content depending on whether they are in full-screen
mode. We'll be able to remove that code now, since this new generic mechanism
takes care of that too.
But we will need this more general mechanism for cases where views truncate
their content to the view width; we'll add one example for that later in this
branch.
refreshWorktrees re-renders the branches view, because the branches view shows
worktrees against branches. This means that when both BRANCHES and WORKTREES are
requested to be refreshed, the branches view would be rendered twice in short
succession. This causes an ugly visual glitch when force-pushing a branch,
because when pushing is done, we would see the ↑4↓9 status come back from under
the Pushing status for a brief moment, to be replaced with a green checkmark a
moment later.
Fix this by including the worktree refresh in the branches refresh when both are
requested. This means that the two are no longer running in parallel for an
async refresh, but hopefully that's not so bad.
When pulling/pushing/fast-forwarding a branch, show this state in the branches
list for that branch for as long as the operation takes, to make it easier to
see when it's done (without having to stare at the status bar in the lower
left).
This will hopefully help with making these operations feel more predictable, now
that we no longer show a loader panel for them.
Very similar to WithWaitingStatus, except that the status is shown in a view
next to the affected item, rather than in the status bar.
Not used by anything yet; again, committing separately to get smaller commits.
This is not a complete fix, but it's good enough to fix the spurious test
failures of submodule/reset.go. We have some vague hope to fix this in a more
sustainable way by somehow improving our concurrency model fundamentally, but
that's a more long-term undertaking, and it's annoying that this test fails so
often, so let's fix it in this way for now.
A new gui config flag 'portraitMode':<string> is added to influence when
LazyGit stacks its UI components on top of one another.
The accepted values are 'auto', 'always', 'never'.
'auto': enter portrait mode when terminal becomes narrow enough
'always': always use portrait mode unconditional of the terminal
dimensions
'never': never use portraid mode
Signed-off-by: Louis DeLosSantos <louis.delos@gmail.com>
Previously there was no way to render a view's search status without also moving the cursor
to the current search match. This caused issues where we wanted to display the status
after leaving the view and coming back, or when beginning a new search from within the
view.
This commit separates the two use cases so we only move the cursor when we're actually
selecting the next search match
Now that we no longer show it in a loader panel, but in the app status view,
it's awkwardly long (the loading animation is much further to the right than for
other waiting status texts). Hopefully seeing just "Fast-forwarding <branch>" is
enough to be able to tell what's happening.
We do this for two reasons:
- when popping up a credentials prompt, it looks distracting if the waiting
status keeps spinning while the user is typing the password
- the task that updates the waiting status periodically would keep the program
busy, so integration tests would wait forever for the program to become idle
again
This can be useful when you know that a cherry-picked commit would conflict at
the tip of your branch, but doesn't at the beginning of the branch (or
somewhere in the middle). In that case you want to be able to edit the commit
before where you want to insert the cherry-picked commits, and then paste to
insert them into the todo list at that point.
This is useful to disable items that are not applicable right now because of
some condition (e.g. the "delete branch" menu item when the currently
checked-out branch is selected).
When a DisabledReason is set on a menu item, we
- show it in a tooltip (below the regular tooltip of the item, if it has one)
- strike through the item's key, if it has one
- show an error message with the DisabledReason if the user tries to invoke the
command
Add co-author to commits
Add addCoAuthor command for commits
- Implement the `addCoAuthor` command to add co-authors to commits.
- Utilize suggestions helpers to populate author names from the suggestions list.
- Added command to gui at `LocalCommitsController`.
This commit introduces the `addCoAuthor` command, which allows users to easily add co-authors to their commits. The co-author names are populated from the suggestions list, minimizing the chances of user input errors. The co-authors are added using the Co-authored-by metadata format recognized by GitHub and GitLab.
This should already have been done when adding the "View divergence from
upstream" command, but now we're going to add yet another item to the menu that
is unrelated to setting or unsetting the upstream.
This fixes two minor problems with the prompts:
1. When pressing shift-A in the local commits view, it would first prompt
whether to stage all files, and then it would prompt whether to amend the
commit at all. This doesn't make sense, it needs to be the other way round.
2. When pressing shift-A on the head commit in an interactive rebase, we would
ask whether they want to amend the last commit, like when pressing shift-A in
the files view. While this is technically correct, the fact that we're
amending the head commit in this case is just an implementation detail, and
from the user's point of view it's better to use the same prompt as we do for
any other commit.
To fix these, we remove the confirmation panel from AmendHelper.AmendHead() and
instead add it at the two call sites, so that we have more control over this.
This encapsulates the logic to make sure we have something to commit; which is
to
- auto-stage all files if no files are staged and the SkipNoStagedFilesWarning
config is on
- otherwise, prompt the user whether they want to stage all files
- error out if we don't have any files at all
Of these, the first one was only done when committing with the built-in commit
message panel; there's no reason why it shouldn't also be done when committing
with the editor, or when amending, and now it is.
It implemented this because it wants to do custom truncation of the ref name;
however, we can achieve the same thing by passing the truncated ref name to our
DynamicTitleBuilder, which was previously unused.
We'll make some changes to how the display strings are rendered, so it helps to
have this code only once. This also fixes the problem that contexts using
refreshViewportOnChange weren't able to use column alignments so far. We didn't
need this yet, but it's just nice if everything works. :)
When editing a commit, the index of the current commit can change; for example,
when merge commits are involved, or when working with stacked branches where
"update-ref" commands may be added above the selected commit.
Reselect the current commit after pressing "e"; this requires doing the refresh
blocking on the main thread. (Another option that I considered was to use a
SYNC refresh, and then select the new line with an OnUIThread inside the Then
function. This also works, but results in a very visible lag.)
Only do this when an onSwitchToEditor function is actually provided. For the
"Move patch into new commit" command we don't, because it isn't totally
straightforward in that case.
This is useful for when you begin to type the message in lazygit's commit panel,
and then realize that you'd rather use your editor's more powerful editing
capabilities. Pressing <c-o> will take you right there.
When navigating in the commits view to a line that is out of view (e.g. by
pressing , or . to scroll by page, or < or > to scroll to the top or bottom),
the commit graph was not correctly highlighted. Fix this by rerendering the
viewport in this case.
It determines the yellow/red status by getting the merge-base between the
current branch and its upstream; while we're rebasing, the current branch is
HEAD, so it tried to get the merge-base between HEAD and HEAD{u}, which doesn't
work. Fix this by passing the name of the checked-out branch separately.
Previously, when pressing right-arrow when the cursor is already in the last
hunk, it would jump back to the beginning of that hunk. This can be confusing if
the hunk is long, maybe the start of the hunk is already scrolled off the top of
the window, and then pressing right-arrow actually scrolls *backwards*, which is
counter-intuitive. It's better to do nothing in this case.
Same for left-arrow when the cursor is already in the first hunk, although here
the problem is not so severe (unless diff context was increased by a huge
amount, and the start of the first hunk is scrolled off the bottom of the
window).
Previously, the current line was only moved as much as necessary so that it's in
view again. This had the problem that when jumping downwards from hunk to hunk
with the right-arrow key, only the first line of the new hunk was shown at the
bottom of the window. I prefer to put the selected line in the middle of the
view in this case, so that I can see more of the newly selected hunk.
This has the consequence that when scrolling through the view line by line using
down-arrow, the view jumps by half a screen whenever I reach the bottom. I can
see how some users might be opposed to this change, but I happen to like it too,
because it allows me to see more context of what's ahead.
This also fixes a bug where after the rebase each commit in the commits view had a tick against it because we hadn't
refreshed the view since the base commit was no longer marked
If the command used by OSCommand.OpenLink fails, lazygit crashes. With this change, if the OpenLink command fails, lazygit just shows a dialog inviting the user to visit the relevant URL.
Fixes#2882
This was on oversight on my part: I assumed that the --work-tree arg was
always intended for use with linked worktrees which have a .git file
pointing back to the repo.
I'm honestly confused now: seems like there are three kinds of worktrees:
* the main worktree of a non-bare repo
* a linked worktree (with its own gitdir in the repo's worktrees/ dir)
* a random folder which you specify as a worktree with the --work-tree arg
I'm pretty sure the --work-tree arg is only intended to be used with this
third kind or workree
... and import stefanhaller's tcell fork for real rather than just replacing it
This solves the problem that people trying to
"go install github.com/jesseduffield/lazygit@latest" would get the error
go: github.com/jesseduffield/lazygit@latest (in github.com/jesseduffield/lazygit@v0.40.0):
The go.mod file for the module providing named packages contains one or
more replace directives. It must not contain directives that would cause
it to be interpreted differently than if it were the main module.
The proper fix is to actually have these two functions share code,
or for views to be able to manage their own heights based on their contents.
But I want to get this out for the sake of a Lazygit Anniversary release.
Now that we refresh upon focus, we can scrap this file watching code.
Stefan says few git UIs use file watching, and I understand why: the
reason this code was problematic in the first place is that watching
files is expensive and if you have too many open file handles that
can cause problems.
Importantly: this code that's being removed was _already_ dead.
When opening lazygit with `lazygit log` the worktrees view was appearing in front of the files view.
This is because it had higher precedence than the files view in the ordered view mapping, and
that was because it originally was in the branches window so it was further down the list.
The reason this didn't cause issues on typical startup is that the files context is activated at the
start so it is brought to the front.
I've been thinking about this for a while: I think it looks really cool if nuking your working tree
actually results in a nuke animation.
So I've added an opt-out config for it
This allows to do the equivalent of "git rebase --onto <target> <base>", by
first marking the <base> commit with the new command, and then selecting the
target branch and invoking the usual rebase command there.
We are in the outsideFilterModeBindings section here; all of these handlers are
wrapped in a OutsideFilterMode guard in a loop below. No need to add one
manually here.
It's tricky to get this right for reflog commits wrt what's the current branch
for each one; so just disable it entirely here, it's probably not something
anybody needs here.
We want to mark all local branch heads with a "*" in the local commits panel, to
make it easier to see how branches are stacked onto each other. In order to not
confuse users with "*" markers that they don't understand, do this only for the
case where users actually use stacked branches; those users are likely not going
to be confused by the display. This means we want to filter out a few branch
heads that shouldn't get the marker: the current branch, any main branch, and
any old branch that has been merged to master already.
The model will be used for logic, so the full hash is needed there; a shortened
hash of 8 characters might be too short to be unique in very large repos. If
some view wants to display a shortened hash, it should truncate it at
presentation time.
We've been sometimes using lo and sometimes using my slices package, and we need to pick one
for consistency. Lo is more extensive and better maintained so we're going with that.
My slices package was a superset of go's own slices package so in some places I've just used
the official one (the methods were just wrappers anyway).
I've also moved the remaining methods into the utils package.
In the presentation layer, when showing branches, we'll show worktrees against branches if they're
associated. But there was a race condition: if the worktree model was refreshed after the branches model,
it wouldn't be used in the presentation layer when it came time to render the branches.
A better solution would be to have some way of signalling that a particular context needs to be refreshed
and after all the models are done being refreshed, we then refresh the contexts. This will prevent
double-renders
When switching worktrees (which we can now do via the branch view) we re-layout the windows and their views.
We had the worktree view ahead of the file view based on the Flatten() method in context.go, because it used
to be associated with the branches panel.
We want to be using forward slashes everywhere internally, so if we get a path from windows
we should immediately convert it to use forward slashes.
I'm leaving out the recent repos list because that would require a migration
There are quite a few paths you might want to get e.g. the repo's path, the worktree's path,
the repo's git dir path, the worktree's git dir path. I want these all obtained once and
then used when needed rather than having to have IO whenever we need them. This is not so
much about reducing time spent on IO as it is about not having to care about errors every time
we want a path.
We now always re-use the state of the repo if we're returning to it, and we always reset the windows to their default tabs.
We reset to default tabs because it's easy to implement. If people want to:
* have tab states be retained when switching
* have tab states specific to the current repo retained when switching back
Then we'll need to revisit this
Older versions of git don't support the -z flag in `git worktree list`.
So we're using newlines.
Also, we're not raising an error upon error because that triggers another refresh,
which gets us into an infinite loop
For marking as good or bad, the current commit is pretty much always the one you
want to mark, not the selected. It's different for skipping; sometimes you know
already that a certain commit doesn't compile, for example, so you might
navigate there and mark it as skipped. So in the case that the current commit is
not the selected one, we now offer two separate menu entries for skipping, one
for the current commit and one for the selected.
This can be useful if you want to find the commit that fixed a bug (you'd use
"broken/fixed" instead of "good/bad" in this case), or if you want to find the
commit that brought a big performance improvement (use "slow/fast"). It's pretty
mind-bending to have to use "good/bad" in these cases, and swap their meanings
in your head.
Thankfully, lazygit already had support for using custom terms during the bisect
(for the case that a bisect was started on the command-line, I suppose), so all
that's needed is adding a way to specify them in lazygit.
Previously our synchronous refreshes took far longer because nothing
was happening concurrently. We now run refresh functions concurrently
and use a wait group to ensure they're all done before returning
Notably, the reflog view is taking ages here because it's got a
few thousand lines to write to the view.
In future we should only populate the view's viewport.
On german/french/spanish keyboards, typing [ requires modifier
keys like AltGr, so the `mod==0` condition is wrong.
Fixes#2573
ch != 0 is useless because IsPrint is implemented this way:
if uint32(r) <= MaxLatin1 {
return properties[uint8(r)]&128 != 0
}
with properties[0] set to 1 (so, bit 7 not set)
-> 0 is not printable.
Previously we used a single-line prompt for a tag annotation. Now we're using the commit message
prompt.
I've had to update other uses of that prompt to allow the summary and description labels to
be passed in
Previously, we would only show the authors based on local commits, but sometimes you want to set a commit author
to that of a commit on another branch. Now, so long as you've viewed the branch's commits, the author will appear
as a suggestion.
Previously we applied a right-align on the first column of _all_ menus, even though we really
only intended for it to be on the first column of the keybindings menu (that you get from pressing
'?')
The true issue was that we were focusing the line in the view before it gets resized in the layout function.
This meant if the view was squashed in accordion mode, the view wouldn't know how to set the cursor/origin to
focus the line.
Now we've got a queue of 'after layout' functions i.e. functions to call at the end of the layout function,
right before views are drawn.
The only caveat is that we can't have an infinite buffer so we're arbitrarily capping it at 1000 and dropping
functions if we exceed that limit. But that really should never happen.
It said "Press tab to toggle focus", which is wrong for people who remapped
their togglePanel key binding to something else. Print the actual key binding
instead.
We have a use-case to rebind 'm' to the merge action in the branches panel. There's three ways to handle this:
1) For all global keybindings, define a per-panel key that invokes it
2) Give a name to all controller actions and allow them to be invoked in custom commands
3) Allow checking for merge conflicts after running a custom command so that users can add their own 'git merge' custom command
that matches the in-built action
Option 1 is hairy, Option 2 though good for users introduces new backwards compatibility issues that I don't want to do
right now, and option 3 is trivially easy to implement so that's what I'm doing.
I've put this under an 'after' key so that we can add more things later. I'm imagining other things like being able to
move the cursor to a newly added item etc.
I considered always running this hook by default but I'd rather not: it's matching on the output text and I'd rather something
like that be explicitly opted-into to avoid cases where we erroneously believe that there are conflicts.
Save has been deprecated for a while, push is the recommended way to save a
stash. Push has been available since 2.13, so we can use it without problems.
I've simplifiied the code because it was too complex for the current requirements, and this fixed the misc/initial_open
test which was occasionally failing due to a race condition around busy tasks
The global counter approach is easy to understand but it's brittle and depends on implicit behaviour that is not very discoverable.
With a global counter, if any goroutine accidentally decrements the counter twice, we'll think lazygit is idle when it's actually busy.
Likewise if a goroutine accidentally increments the counter twice we'll think lazygit is busy when it's actually idle.
With the new approach we have a map of tasks where each task can either be busy or not. We create a new task and add it to the map
when we spawn a worker goroutine (among other things) and we remove it once the task is done.
The task can also be paused and continued for situations where we switch back and forth between running a program and asking for user
input.
In order for this to work with `git push` (and other commands that require credentials) we need to obtain the task from gocui when
we create the worker goroutine, and then pass it along to the commands package to pause/continue the task as required. This is
MUCH more discoverable than the old approach which just decremented and incremented the global counter from within the commands package,
but it's at the cost of expanding some function signatures (arguably a good thing).
Likewise, whenever you want to call WithWaitingStatus or WithLoaderPanel the callback will now have access to the task for pausing/
continuing. We only need to actually make use of this functionality in a couple of places so it's a high price to pay, but I don't
know if I want to introduce a WithWaitingStatusTask and WithLoaderPanelTask function (open to suggestions).
We had a race condition due to refreshing branches in two different places, one which refreshed reflog commits
beforehand. The race condition meant that upon load we wouldn't see recency values (provided by the reflog commits)
against the branches
Turns out we're just running our refresh functions one after the other which isn't ideal but we can fix that separately.
As it stands this wait group isn't doing anything.
I don't know if this is a hack or not: we run a git command and increment the pending action
count to 1 but at some point the command requests a username or password, so we need to prompt
the user to enter that. At that point we don't want to say that there is a pending action,
so we decrement the action count before prompting the user and then re-increment it again afterward.
Given that we panic when the counter goes below zero, it's important that it's not zero
when we run the git command (should be impossible anyway).
I toyed with a different approach using channels and a long-running goroutine that
handles all commands that request credentials but it feels over-engineered compared to this
commit's approach.
The remote branches controller was using its own escape method meaning it didn't go through the flow of cancelling
an active filter. It's now using the same approach as the sub-commits and commit-files contexts: defining a parent
context to return to upon hittin escape.
Given that we now persist search/filter states even after a side context loses focus, we need to make it really
clear to the user that the context is currently being searched/filtered
This is a pickle: initially I wanted it so that a filter would cancel automatically if the current context lost focus.
But there are situations where you want to retain the focus, e.g. when a popup appears, or when you view the commits
of a branch. The issue is that when you view the commits of a branch, the branches context is removed from the context
stack. Even if this were not the case, you could imagine going branches -> sub-commits -> files -> sub-commits, where
in that case branches would definitely be off the stack upon navigating to the files context.
So because I'm too lazy to find a proper solution to this problem, I'm just making it so that filters in side contexts
are retained unless explicitly cancelled.
There's another edge case this commit handles which is that if I'm in the sub-commits context via the branches context
and start a search, then navigate to the reflog context and hit enter to get to the sub-commits context again, I need
to cancel the search before I switch. Likewise with the commit files context.
For consistency with the previous commit.
Note that this menu entry is used both for unstaged and for staged changes, and
for staged changes it is not quite accurate, as we are not discarding changes in
that case (just unstaging them). Not sure it's worth fixing this; it's still
better than "Delete", anyway.
The title was saying "Unstage lines", which was just wrong. The text said
"Delete lines", which can be seen as a bit misleading; we are only discarding
the changes to the selected lines, not deleting the lines themselves.
For consistency, rename the config variable skipUnstageLineWarning accordingly.
When stopping in a rebase because of a conflict, it is nice to see the commit
that git is trying to apply. Create a fake todo entry labelled "conflict" for
this, and show the "<-- YOU ARE HERE ---" string for that one (in red) instead
of for the real current head.
We don't actually use it to do map lookups; we still iterate over it in the same
way as before. However, using a map makes it easier to patch elements; see the
next commit.
We use CommitFilesController also for the files of commits that we show
elsewhere, e.g. for branch commits, tags, or stashes. It doesn't make sense to
discard changes from those (for stashes it might be possible to implement it
somehow, but that would be a new feature), so we disallow it unless we are in
the local commits panel.
Discarding changes to an entire directory doesn't quite work correctly in all
cases; for example, if the current commit added files to the directory (but the
directory existed before) then those files won't be removed.
It might be possible to fix the command so that these cases always work for
directories, but I don't think it's worth the effort (you can always use a
custom patch for that), so let's display an error for now.
I don't know why we were setting the initial context to CurrentSideContext
and not just CurrentContext in the first place. If there is no current context
in either case it'll default to the files context. So the only issue is if
we anticipated that some random context would be focused and we didn't want to
activate that. But I can't think of any situation where that would happen.
The root commit is special in that it has no parents. So we need to add a pipe that's headed for a commit
that doesn't actually exist i.e. the mythical empty tree commit. We're using the actual hash of that
pseudo-commit, but it's not being read anywhere.
The menuFromCommand option is a little complicated, so I'm adding an easy way to just use the command output directly,
where each line becomes a suggestion, as-is.
Now that we support suggestions in the input prompt, there's less of a need for menuFromCommand, but it probably still
serves some purpose.
In future I want to support this filter/valueFormat/labelFormat thing for suggestions too. I would like to think a little more
about the interface though: is using a regex like we currently do really the simplest approach?
We have not been good at consistent casing so far. Now we use 'Sentence case' everywhere. EVERYWHERE.
Also Removing 'Lc' prefix from i18n field names: the 'Lc' stood for lowercase but now that everything
is in 'Sentence case' there's no need for the distinction.
I've got a couple lower case things I've kept: namely, things that show up in parentheses.
Go really doesn't like us doing anything inheritance-y: it does not support open recursion meaning
it's really hard to re-use code. As such, here we're falling back to conditional logic.
This fixes an issue where our ListContextTrait was calling FocusLine which was intended to be
overridden by ViewportListContextTrait, but the subclassed function wasn't being called. I'm
not actually sure how this went wrong given that it was working fine in the past, but at any rate,
the new code is easy to follow.
By constructing an arg vector manually, we no longer need to quote arguments
Mandate that args must be passed when building a command
Now you need to provide an args array when building a command.
There are a handful of places where we need to deal with a string,
such as with user-defined custom commands, and for those we now require
that at the callsite they use str.ToArgv to do that. I don't want
to provide a method out of the box for it because I want to discourage its
use.
For some reason we were invoking a command through a shell when amending a
commit, and I don't believe we needed to do that as there was nothing user-
supplied about the command. So I've switched to using a regular command out-
side the shell there
If a given menu item has an associated keybinding of 'enter', hitting enter won't actually execute
that item unless your cursor is on it. This creates confusion, and so we're going to use a strikethrough
style to communicate that the keybinding is reserved for something else.
The reason for this is that now our labels for navigation keybindings are larger so they
take up more realestate. It's not the kind of thing a user needs to be told anyway,
anybody is going to try out hjkl and the arrow keys when a TUI opens up.
We could map from <up> to the single character up unicode rune but given you can rebind this stuff
I'd rather keep it simple
The option doesn't have any affect in these views, so we don't need to toggle it
here. But the problem was the HandleFocus call at the end: this would activate
the wrong view, so we need to avoid it here.
Show an error if the user tries to turn the option on, to let them know that it
doesn't work here.
It's not possible to reliably stage things into a custom patch when "ignore
whitespace" is on, so always treat it as off here (like we do in the staging
panel).
It looks like this is a regression that was introduced in 8edad826ca.
Our refresh code may try to push a context. It does this in two places:
1) when all merge conflicts are resolved, we push a 'continue merge?' confirmation context
2) when all conflicts of a given file are resolved and we're in the merge conflicts context,
we push the files context.
Sometimes we push the confirmation context and then push the files context over it, so the user
never sees the confirmation context.
This commit fixes the race condition by adding a check to ensure that we're still in the
merge conflicts panel before we try escaping from it
I don't see a reason why this restriction to have the selection be always
visible was necessary. Removing it has two benefits:
1. Scrolling a list view doesn't change the selection. A common scenario: you
look at one of the commits of your current branch; you want to see the how
many'th commit this is, but the beginning of the branch is scrolled off the
bottom of the commits panel. You scroll down to find the beginning of your
branch, but this changes the selection and shows a different commit now - not
what you want.
2. It is possible to scroll a panel that is not the current one without changing
the focus to it. That's how windows in other GUIs usually behave.
We now refresh the staging panel when doing an unscoped refresh, so that if we commit from the staging panel we escape
back to the files panel if need be. But that causes flickering when doing an unscoped refresh from other contexts,
because the refreshStagingPanel function assumes that the staging panel has focus. So we're adding a guard at the top
of that function to early exit if we don't have focus.
When cycling history, we want to make it so that upon returning to the original prompt, you get your text back.
Importantly, we don't want to use the existing preservedMessage field for that because that's only for preserving
a NEW commit message, and we don't want the history stuff of the commit reword flow to overwrite that.
When we use the one panel for the entire commit message, its tricky to have a keybinding both for adding a newline and submitting.
By having two panels: one for the summary line and one for the description, we allow for 'enter' to submit the message when done from the summary panel,
and 'enter' to add a newline when done from the description panel. Alt-enter, for those who can use that key combo, also works for submitting the message
from the description panel. For those who can't use that key combo, and don't want to remap the keybinding, they can hit tab to go back to the summary panel
and then 'enter' to submit the message.
We have some awkwardness in that both contexts (i.e. panels) need to appear and disappear in tandem and we don't have a great way of handling that concept,
so we just push both contexts one after the other, and likewise remove both contexts when we escape.
This begins a big refactor of moving more code out of the Gui struct into contexts, controllers, and helpers. We also move some code into structs in the
gui package purely for the sake of better encapsulation
We now refresh the staging panel when doing an unscoped refresh, so that if we commit from the staging panel we escape
back to the files panel if need be. But that causes flickering when doing an unscoped refresh from other contexts,
because the refreshStagingPanel function assumes that the staging panel has focus. So we're adding a guard at the top
of that function to early exit if we don't have focus.
When cycling history, we want to make it so that upon returning to the original prompt, you get your text back.
Importantly, we don't want to use the existing preservedMessage field for that because that's only for preserving
a NEW commit message, and we don't want the history stuff of the commit reword flow to overwrite that.
When we use the one panel for the entire commit message, its tricky to have a keybinding both for adding a newline and submitting.
By having two panels: one for the summary line and one for the description, we allow for 'enter' to submit the message when done from the summary panel,
and 'enter' to add a newline when done from the description panel. Alt-enter, for those who can use that key combo, also works for submitting the message
from the description panel. For those who can't use that key combo, and don't want to remap the keybinding, they can hit tab to go back to the summary panel
and then 'enter' to submit the message.
We have some awkwardness in that both contexts (i.e. panels) need to appear and disappear in tandem and we don't have a great way of handling that concept,
so we just push both contexts one after the other, and likewise remove both contexts when we escape.
This broke with 40f6767cfc77; the symptom is that starting lazygit with a git
arg (e.g. "lazygit log") wouldn't activate the requested panel correctly. While
it would show in the expanded view, it didn't have a green frame, and keyboard
events would go to the files panel.
This fixes two problems with the "amend commit with staged changes" command:
1. Amending to a fixup commit didn't work (this would create a commmit with the
title "fixup! fixup! original title" and keep that at the top of the branch)
2. Unrelated fixup commits would be squashed too.
The added integration test verifies that both of these problems are fixed.
We already show "merge" todo entries when starting an interactive rebase with
--rebase-merges outside of lazygit. Changing the type of a merge entry to "pick"
or "edit" doesn't make sense and shouldn't be allowed. Earlier in this branch we
have started to show "update-ref" entries, these can't be changed either (they
can be moved, though).
You might argue that it should be possible to change them to "drop", but in the
case of "update-ref" this doesn't make sense either, because "drop" needs a Sha
and we don't have one here. Also, you would then be able to later change it back
to "pick", so we would have to remember that this isn't allowed for this
particular drop entry; that's messy, so just disallow all editing.
It used to work on the assumption that rebasing commits in lazygit's model
correspond one-to-one to lines in the git-rebase-todo file, which isn't
necessarily true (e.g. when users use "git rebase --edit-todo" at the custom
command prompt and add a "break" between lines).
This is useful when working with stacked branches, because you can now move
"pick" entries across an update-ref command and you can tell exactly which
branch the commit will end up in.
It's also useful to spot situations where the --update-refs option didn't work
as desired. For example, if you duplicate a branch and want to rebase only one
of the branches but not the other (maybe for testing); if you have
rebase.updateRefs=true in your git config, then rebasing one branch will move
the other branch along. To solve this we'll have to introduce a way to delete
the update-ref entry (maybe by hitting backspace?); this is out of scope for
this PR, so for now users will have to type "git rebase --edit-todo" into the
custom command prompt to sort this out.
We will also have to prevent users from trying to turn update-ref commands into
other commands like "pick" or "drop"; we'll do this later in this branch.
The main reason for doing this (besides the reasons given for Status in the
previous commit) is that it allows us to easily convert from TodoCommand to
Action and back. This will be needed later in the branch. Fortunately,
TodoCommand is one-based, so this allows us to add an ActionNone constant with
the value 0.
This is unrelated to the changes in this PR, but since we are doing the same
thing for the commit.Action field in the next commit, it makes sense to do it
for Status too for consistency. Modelling this as an enum feels more natural
than modelling it as a string, since there's a finite set of possible values.
And it saves a little bit of memory (not very much, since none of the strings
were heap-allocated, but still).
We print this to the terminal after lazygit quits rather than showing it in a
panel at startup, so as to not annoy people too much. Hopefully it will still be
prominent enough this way.
The "open" command is supposed to behave in the same way as double-clicking a
file in the Finder/Explorer. The concept of jumping to a specific line in the
file doesn't make sense for this; use "edit" instead.
Previously we would have tried to do the rebase, resulting in a long and
somewhat cryptic error message from git; now we check ourselves and show a less
intimidating message.
We refresh the view after reading just enough to fill it, so that we see the
initial content as quickly as possible, but then we continue reading enough
lines so that we can tell how long the scrollbar needs to be, and then we
refresh again. This can result in slight flicker of the scrollbar when it is
first drawn with a bigger size and then jumps to a smaller size; however, that's
a good tradeoff for a solution that provides both good speed and accuracy.
