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.
- **PR Description**
This PR fixes a problem with lazygit locking up completely when there's
a commit whose subject line is longer than approximately 65500
characters.
Fixes#3529.
We are going to truncate overly long lines returned from git log, and the most
likely field that is going to make the line too long is the subject; so we must
put it last, otherwise we'd end up with not enough fields to split when it's too
long.
It might not be obvious from the diff what's happening to the mock command
output in the test: it didn't have the divergence field (">") at all, which was
kind of a bug. It didn't matter for these tests though, because we are not
testing the divergence here, and our production code happens to be resilient
against it missing. But now we must add the ">" field before the subject.
Scanners can return errors (e.g. ErrTooLong), and if we don't handle it, the
cmd.Wait() call below will block forever because nobody drains the command's
output.
This happens for CommitLoader.GetCommits when there's a commit whose subject
line is longer than approx. 65500 characters; in that case, lazygit would lock
up completely. With this fix it remains usable, but the commit list is truncated
before the bad commit, which is not good enough. We'll improve that in the
remaining commits of this branch.
- **PR Description**
Clicking in the status side panel should activate it; also, clicking on
the repo name should bring up the recent repositories menu, and clicking
on the "(rebasing)" text should bring up the rebase options menu. All of
this was broken for a long time, since somewhere around the big
refactoring of March 2023. (The exact commit where it broke is hard to
bisect, since many of the commits in that area either don't compile or
crash at startup...)
I'm fixing this not because I think it's super important functionality
(nobody seems to have missed it for over a year), but because I have to
touch this code in another PR, and noticed that it wasn't working.
- **Please check if the PR fulfills these requirements**
* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [x] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [ ] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] Docs (specifically `docs/Config.md`) have been updated if
necessary
* [x] You've read through your own file changes for silly mistakes etc
Deadlock reporting broke in e1ceb6892a (last September); 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.
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.
- **PR Description**
In the "View divergence from upstream" view we have so far disabled the
commit graph because it was too difficult to implement properly. I
really miss it though, so here's a PR that enables it there, too.
For feature branches it is not essential, because these usually don't
contain merges and the graph is a trivial line. However, for the master
branch against its upstream it is useful too see how many PRs were
merged since you last fetched it, and the graph helps a lot with that.
Also, when we implement #3536 it will be very useful there, too.
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.
- **PR Description**
Support `git.paging.externalDiffCommand` config in diffing mode (i.e.
when showing the diff between two commits using `W`).
Fixes#3518.
- **PR Description**
The rebase.updateRefs feature of git is very useful to rebase a stack of
branches and keep everything nicely stacked; however, it is usually in
the way when you make a copy of a branch and want to rebase it "away"
from the original branch in some way or other. For example, the original
branch might sit on main, and you want to rebase the copy onto devel to
see if things still compile there. Or you want to do some heavy history
rewriting experiments on the copy, but keep the original branch in case
the experiments fail. Or you want to split a branch in two because it
contains two unrelated sets of changes; so you make a copy, and drop
half of the commits from the copy, then check out the original branch
and drop the other half of the commits from it.
In all these cases, git's updateRefs feature insists on moving the
original branch along with the copy in the first rebase that you make on
the copy. I think this is a bug in git, it should create update-ref
todos only for branches that point into the middle of your branch
(because only then do they form a stack), not when they point at the
head (because then it's a copy). I had a long discussion about this on
the git mailing list [1], but people either don't agree or don't care
enough.
So we fix this on our side: whenever we start a rebase for whatever
reason, be it interactive, non-interactive, or behind-the-scenes, we
drop any update-ref todos that are at the very top of the todo list,
which fixes all the above-mentioned scenarios nicely.
I will admit that there's one scenario where git's behavior is the
desired one, and the fix in this PR makes it worse: when you create a
new branch off of an existing one, with the intention of creating a
stack of branches, but before you make the first commit on the new
branch you realize some problem with the first branch (e.g. a commit
that needs to be reworded or dropped). It this case you do want both
branches to be affected by the change. In my experience this scenario is
much rarer than the other ones that I described above, and it's also
much easier to recover from: just check out the other branch again and
hard-reset it to the rebased one.
[1]
https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/
The rebase.updateRefs feature of git is very useful to rebase a stack of
branches and keep everything nicely stacked; however, it is usually in the way
when you make a copy of a branch and want to rebase it "away" from the original
branch in some way or other. For example, the original branch might sit on main,
and you want to rebase the copy onto devel to see if things still compile there.
Or you want to do some heavy history rewriting experiments on the copy, but keep
the original branch in case the experiments fail. Or you want to split a branch
in two because it contains two unrelated sets of changes; so you make a copy,
and drop half of the commits from the copy, then check out the original branch
and drop the other half of the commits from it.
In all these cases, git's updateRefs feature insists on moving the original
branch along with the copy in the first rebase that you make on the copy. I
think this is a bug in git, it should create update-ref todos only for branches
that point into the middle of your branch (because only then do they form a
stack), not when they point at the head (because then it's a copy). I had a long
discussion about this on the git mailing list [1], but people either don't agree
or don't care enough.
So we fix this on our side: whenever we start a rebase for whatever reason, be
it interactive, non-interactive, or behind-the-scenes, we drop any update-ref
todos that are at the very top of the todo list, which fixes all the
above-mentioned scenarios nicely.
I will admit that there's one scenario where git's behavior is the desired one,
and the fix in this PR makes it worse: when you create a new branch off of an
existing one, with the intention of creating a stack of branches, but before you
make the first commit on the new branch you realize some problem with the first
branch (e.g. a commit that needs to be reworded or dropped). It this case you do
want both branches to be affected by the change. In my experience this scenario
is much rarer than the other ones that I described above, and it's also much
easier to recover from: just check out the other branch again and hard-reset it
to the rebased one.
[1]
https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/
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 was added in 2018 (700f8c7e79), but I don't know for what purpose. It just
took me 15 minutes to figure out why my new file todo.go wasn't added, so I'm
removing this entry as I find it more harmful than helpful.
- **PR Description**
Simplify and canonicalize error handling across the code base.
Previously it was important to make sure that errors don't bubble up
into gocui, because it would panic there; so we had to show errors in
error panels in the right places (in controller code, usually). This is
error-prone because it's easy to forget (see #3490 for an example);
also, it's not always obvious where in the call chain the error panel
needs to be shown. Most of the time it's in controller code, but we had
plenty of calls to `Error()` in helpers, and I remember that I found
this very confusing when I was new to the code base.
Change this so that you can simply return errors everywhere. The
functions to show an error message manually have been removed for
clarity.
I tried to structure the commits so that you can review them one by one.
Closes#3491.
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.
- **PR Description**
Seems that there's a problem in the Stdout/Stderr/Stdin stream vars
assignments,
probably copy-paste issue.
If this is intentional, I suggest adding an explanation on why `Stderr`
-> `Stdout` intentionally to avoid other PRs and confused people around
:)
Thanks!
- **PR Description**
It seems that setting TERM=dumb breaks compatibility with curses
subprocess in Go, especially with `pinentry-curses`
This fixes#3308
- **PR Description**
Add `spinner` config section to the user config. The section has 2
options:
- `frames` - list of strings that are iterated in the spinner animation
- `rate` - spinner's rate in milliseconds
Closes#3435
- **PR Description**
Migrate users' config files to change all `null` keybindings to
`"<disabled>"`. Then remove our yaml fork, since that was the only
reason we were using it.
Fixes#3458.
Unfortunately the migration code requires yaml v3, but our yaml fork is based on
v2, so we need to import both in app_config.go in this commit, which is ugly. We
can clean this up in the next commit.
- **PR Description**
This is especially helpful for the breaking changes popup, which has a
link to the release notes, but it could also be useful for other panels
that display some warning or error with a link to more information.
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.
- **PR Description**
For people who have the log.showsignature git config set to true, trying
to reword a signed commit would put the signature verification into the
subject field and the commit subject into the description field of the
commit message panel. Amending commits, adding co-authors to a commit,
and copying a commit message to the clipboard would all be broken in a
similar way.
Slightly related is #1911, but back then it was more about performance
than wrong behavior, it seems.
Fixes#3425.
For people who have the log.showsignature git config set to true, trying to
reword a signed commit would put the signature verification into the subject
field and the commit subject into the description field of the commit message
panel. Amending commits, adding co-authors to a commit, and copying a commit
message to the clipboard would all be broken in a similar way.
- **PR Description**
Make the "Add to .git/info/exclude" command work correctly in a worktree
or in a submodule. Previously it would result in an error message.
Fixes#3427.
Using the "Add to .git/info/exclude" in a worktree results in an error message,
as the test shows. The same would happen in a submodule, but I'm not adding an
extra test for that, as the circumstances are the same.
- **PR Description**
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.
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.
- **PR Description**
It would crash when some keybindings are set to null, and the filter
string is such that only those keybindings remain visible.
Fixes#3449.
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.
- **PR Description**
Deleting an update-ref todo in an interactive rebase now behaves as
expected (i.e. it leaves the branch referenced by the update-ref
untouched). Previously it would have deleted the branch referenced by
the update-ref todo when the rebase was continued. See #3418 for more
details.
Fixes#3418.
It is a bad idea to read a git-rebase-todo file, remove some update-ref todos,
and write it back out behind git's back. This will cause git to actually remove
the branches referenced by those update-ref todos when the rebase is continued.
The reason is that git remembers the refs affected by update-ref todos at the
beginning of the rebase, and remembers information about them in the file
.git/rebase-merge/update-refs. Then, whenever the user performs a "git rebase
--edit-todo" command, it updates that file based on whether update-ref todos
were added or removed by that edit. If we rewrite the git-rebase-todo file
behind git's back, this updating doesn't happen.
Fix this by not updating the git-rebase-todo file directly in this case, but
performing a "git rebase --edit-todo" command where we set ourselves as the
editor and change the file in there. This makes git update the bookkeeping
information properly.
Ideally we would use this method for all cases where we change the
git-rebase-todo file (e.g. moving todos up/down, or changing the type of a
todo); this would be cleaner because we wouldn't mess with git's private
implementation details. I tried this, but unfortunately it isn't fast enough.
Right now, moving a todo up or down takes between 1 and 2ms on my machine;
changing it to do a "git rebase --edit-todo" slows it down to over 100ms, which
is unacceptable.
In the test we simply removed the update-ref todo but didn't make any other
changes to the todos. This should really have kept everything the way it was,
including the other branch head. The fact that the star was gone was really
because of the bug that we are going to fix later in the branch.
Change the test so that it also makes a change before the update-ref todo; this
way we test that the star is gone because we deleted the update-ref, not because
of the bug.
To guard against the bug, we add another assertion for the branches view to test
that both branches are still there. This currently fails.
- **PR Description**
Running WSL without a container would be treated as native linux,
causing problems at it would then attempt to use `xdg-open`. This was
caused by `isContainer()` always returning true due to some dubious
conditionals. These have been removed.
The env-var check seems to not be used by lazygit, nor any common
containers, and therefore appears to only exist to manually tell lazygit
to behave as if it were inside of a container. This functionality has
been kept, but the env-var has been changed to be all uppercaps as to
comply with the POSIX standard.
Fixes#2757
Bug introduced in 4d78d76
- **PR Description**
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).
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.
- **PR Description**
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.
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.
- **PR Description**
This makes it possible to select a range of files (either in the files
panel, or in the commit-files panel), and hit `e` to open them all at
once in the editor.
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 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.
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.
- **PR Description**
When lazygit suspends itself to the background to run an external
command, and the command returns a non-zero exit code, always show the
prompt for pressing enter to return to lazygit, even if the
`promptToReturnFromSubprocess` is set to `false`. The rationale is that
if the process returned an error, it likely also printed some error
message to the console that users will always want to read.
I was considering turning the `promptToReturnFromSubprocess` config into
an enum with values `never`, `onlyOnError`, `always`, but then I felt
that `onlyOnError` is really the only sensible choice for people who
have it set to `false` now, so I just hard-coded that.
Stefan Haller
