- **PR Description**
For my current work on better cherry-picking/reverting, git versions
older than 2.22 cause considerable headache. Apparently they have issues
with `git cherry-pick --continue` and `git cherry-pick --skip` which I
would prefer not to have to work around.
The last time [we
discussed](https://github.com/jesseduffield/lazygit/issues/2457#issuecomment-1436767849)
the question of bumping the minimum version from 2.20 to 2.22, we
decided not to because 2.22 is 6 months newer. That was two years ago
though, so now it should be fine, I guess.
I usually use something like
PATH=~/git-versions/2.22.0/bin:$PATH ./scripts/run_integration_tests.sh
for running integration tests with an older version. This has the problem that
when you specify a version that you don't have locally, it will silently use the
current version. Guard against that by printing the version it is using.
- **PR Description**
We offer an inline merge conflict editor for text conflicts (i.e. where
both sides modify the same section of code). However, there are other
types of conflicts that can't be resolved this way, for example when one
side modifies a file and the other side deletes it. For these cases we
would previously only show `* Unmerged path` in the main view, which
isn't helpful. (Also, for some of these we would split the main view and
show this text both in the unstaged changes and staged changes views,
which is a bit embarrassing.) But more importantly, it was very unclear
how to resolve such a conflict. The only option we had was to discard
the file, which would basically pick "ours" and discard "theirs"; but
there was no way to do the opposite.
This PR improves the situation in two ways:
- it shows elaborate help texts in the main view explaining the
situation (which, in some obscure cases, can be extremely non-obvious,
and a `git status` on the command line doesn't help at all either). For
`UD` and `DU` conflicts we also show the diff of the side that didn't
delete the file; this is usually essential for resolving the conflict
properly, because it's likely that this diff needs to be applied
manually somewhere else.
- when pressing enter, we show a dialog with options to keep the
modified file or to delete it.
A note about terminology: a common way to describe the two sides of a
merge is "ours" and "theirs". I dislike these terms, because while they
work well for merges, they are backwards [for
rebases](https://git-scm.com/docs/git-checkout#Documentation/git-checkout.txt---ours).
I chose to avoid them in this PR, and to use the terms "current" and
"incoming" instead (like in the conflict code lenses in VS Code), which
I think work much better in general; however, they might not be easy to
understand when they occur in the middle of a sentence, so maybe we
should put them in quotes there.
- **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)
* [x] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [ ] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
For the less common conflict types DD, AU, UA, DU, and UD, we would previously
only show "* Unmerged path" in the main view, which isn't helpful. Also, for
some of these we would split the main view and show this text both in the
unstaged changes and staged changes views, which is a bit embarrassing.
Improve this by offering more explanation about what's going on, and what the
most likely way to resolve the situation is for each case.
- **PR Description**
Fix a regression (introduced with the root item PR, #4346) that caused
renamed files to be displayed with their full path in tree view.
While fixing this I noticed that the display of moved files is a bit
confusing; for example, you can't distinguish a file being moved from
the root level into a directory from one that was renamed inside the
directory; see commit message of the first commit for more. I'm not
doing anything about this right now, just fix the regression for now.
Labeled as "ignore-for-release" because it fixes a regression in code
that wasn't released yet.
- **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))
* [ ] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [ ] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
We keep the same commit selected (even though its index changed because of the
added update-ref todo), which is nice; however, the main view shows the diff of
the wrong commit, which is very confusing. I'm suprised that this hasn't been
noticed yet.
The reason why this happens is that we first do the refresh, which includes
re-rendering the main view diff (with the same commit index as before, so the
wrong one), and then we restore the correct commit index but don't render the
main view again.
- **PR Description**
Add bindings for `,` (page up), `.` (page down), `<` or `<home>` (top),
and `>` or `<end>` (bottom), for scrolling long text in confirmation
panels. This is useful for example for git hooks that output a lot of
error text on failure, where the most interesting bit of information is
probably at the end.
I chose not to bind `<pgUp>` and `<pgDown>`, since they are normally
used for scrolling the main view. Which is not a thing when a
confirmation is shown or the Extras panel is focused, so we *could* use
them in these cases, but I thought it might be confusing when they are
used for different things in different contexts.
While we're at it, add the same navigation bindings also to the Extras
panel (i.e. the command log when it has the focus).
Fixes#4372.
- **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))
* [ ] 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))
* [ ] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [ ] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
I can only guess here: maybe they were added to more clearly document the public
interface of the classes? If so, I don't think that works. Developers who are
not familiar with the convention will just add a new public method to the class
without updating the interface.
Apparently this was an attempt at working around go's lack of default arguments,
but it's very unidiomatic and a bit confusing. Make it a normal parameter
instead, so all clients have to pass it explicitly.
- **PR Description**
This fixes the bug that hyperlinks were not clickable if they occurred
in the last line of a confirmation panel.
While we're at it, make them clickable in tooltips too, which is useful
if they contain documentation links (e.g. the tooltip of the `<c-f>`
binding in the Files panel).
- **PR Description**
In version 0.45.0 we started to use an interactive shell for running
shell commands (see #4159). The idea was that this allows users to use
their aliases and shell functions in lazygit without having to do any
additional configuration.
Unfortunately, this hasn't worked out well. For some users this resulted
in lazygit hanging in the background upon trying to return from the
shell command; we tried various fixes for this (see #4126, #4159, and
#4350), but some users still have this problem (e.g. #4320).
Also, starting an interactive shell can be a lot slower than starting a
non-interactive one, depending on how much happens in the `.bashrc` or
`.zshrc` file. For example, on my machine calling `zsh -ic true` takes
600ms, whereas `zsh -c true` takes less than 2ms. This is too high of a
price to pay for using shell aliases, especially when _all_ users have
to pay it, even those who don't care about using their aliases in
lazygit.
This PR reverts all commits related to interactive shells, and instead
introduces a different approach: we let users specify a shell aliases
file that will be sourced before running a command. The downside is that
it doesn't work transparently out of the box, but requires
configuration, and it may also require that users restructure their
shell startup file(s) if they currently only have a single big one. The
advantage is that only users who actually want to use aliases or
functions are affected, and that we can now use this mechanism not only
for shell commands, but also for custom commands and for calling the
editor (some users have asked for this in the past).
There is a section at the end with deprecated settings, and a comment saying
"The following configs are all deprecated". The clipboard-related settings were
accidentally added to that section; they are not deprecated, so move them up to
before that section.
- **PR Description**
This PR addresses https://github.com/jesseduffield/lazygit/issues/3951.
The current rules for discarding submodule changes is that no other
changed item must be also selected. There are some bugs with the current
implementation when submodules are in folders. The filed issue goes into
more detail.
As part of this PR, I also tentatively changed the disabled message
("Range select not supported for submodules" -> "Multiselection not
supported for submodules"). The former was not quite accurate because
you could select a single line (folder) but the reset action still needs
to be disabled (folder contains submodule change and some other change).
Not sure if there is some better phrasing.
The current rules for discarding submodule changes is that no other changed item
must be also selected. There are some bugs with the current implementation when
submodules are in folders.
For example, selecting and discarding a folder with only a nested submodule
change will currently do nothing. The submodule changes should be discarded. The
folder only contains submodule changes so it should be no different than
pressing discard on the submodule entry itself.
Also, I noticed range selecting both the folder and the submodule and then
pressing discard would be incorrectly disallowed.
Renaming a file inside the same directory shows it with its full path in the
tree view, which isn't what we want. We'll fix this in the next commit.
Also adding a few other test cases for moving files; they show that the display
of moved files in tree view isn't ideal. For example, moving file1 from top
level into dir shows it as "R file1 → file1", which isn't distinguishable from
renaming file1 inside dir. I suppose what we would like to have here is
"R ../file1 → file1" or something, but I'll leave that for the future; here I
only want to fix the regression that was introduced with the root item PR.
- **PR Description**
The integration test `stash/rename.go` occasionally fails for me
locally. This PR fixes it.
In 8b8343b8a9 we made a change to run newPtyTask from AfterLayout; this
is needed so that the PTY gets the new, updated view size. However, this
created a race condition for integration tests that select a line in a
list view and then expect the main view to have certain content;
sometimes that content gets rendered too late.
I'm surprised that this didn't cause more tests to fail; right now I
only know of one test that occasionally fails because of this, which is
`stash/rename.go`.
Fix this by moving the AfterLayout to inside newPtyTask, and do it only
when we are actually using a PTY (we don't when no pager is configured,
which is the case for integration tests).
The diff is best viewed with "ignore whitespace" turned on.
In 8b8343b8a9 we made a change to run newPtyTask from AfterLayout; this is
needed so that the PTY gets the new, updated view size. However, this created a
race condition for integration tests that select a line in a list view and then
expect the main view to have certain content; sometimes that content gets
rendered too late.
I'm surprised that this didn't cause more tests to fail; right now I only know
of one test that occasionally fails because of this, which is stash/rename.go.
Fix this by moving the AfterLayout to inside newPtyTask, and do it only when we
are actually using a PTY (we don't when no pager is configured, which is the
case for integration tests).
The diff is best viewed with "ignore whitespace" turned on.
- **PR Description**
* The keybind for moving a commit up in the interactive rebase example
of the README file is incorrect.
I've changed it to the keybind present in current versions of Lazygit
- **PR Description**
When selecting a range of commits by selecting the top one and then
pressing shift-down to create a range, and then pressing enter and
checking out one of the files by pressing `c`, you would get the file
checked out with its state at the bottom end of the range, which is not
what you want; it is expected to check out the file at the state that
the diff shows it changes to. (And if the file was only created in the
middle of that range, trying to check it out would result in an error.)
Fix this by paying attention to a "from-to" range selection, and
checking out the file at the "to" state.
Related to #4420.
