When fetching a remote in the remotes tab, show the status inline (i.e.
in the list row of the remote), like we do when fast-forwarding a branch
in the branches panel.
- **PR Description**
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.
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.
This changes GetRepoPaths() to pull information from `git rev-parse`
instead of effectively reimplementing git's logic for pathfinding. This
change fixes issues with bare repos, esp. versioned homedir use cases,
by aligning lazygit's path handling to what git itself does.
This change also enables lazygit to run from arbitrary subdirectories of
a repository, including correct handling of symlinks, including "deep"
symlinks into a repo, worktree, a repo's submodules, etc.
Integration tests are now resilient against unintended side effects from
the host's environment variables. Of necessity, $PATH and $TERM are the
only env vars allowed through now.
- **PR Description**
I prefer this because I almost never use sticky range selections, but
I'm not sure everybody agrees. 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.
See #3233.
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.
Previously if we marked a line with IsSelected() we would check if it was selected, but
we would not check if other lines were unexpectedly selected. Now, if you use IsSelected(),
we ensure that _only_ the lines you marked as such are the selected lines.
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.
- **PR Description**
Now that branches can be sorted by date, there's a new situation that we
didn't have before: when fetching a branch, its committer date can
change, so it will move up in the list; we need to update the selection
index to follow. This is important for the case that master is behind
its upstream and you want to rebase your checked-out branch onto master:
in that case you would select master, press "f" to fetch, and then press
"r" to rebase onto it. It's very bad if master doesn't stay selected
after fetching.
- **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
<!--
Be sure to name your PR with an imperative e.g. 'Add worktrees view'
see https://github.com/jesseduffield/lazygit/releases/tag/v0.40.0 for
examples
-->
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".
- **PR Description**
Issue: https://github.com/jesseduffield/lazygit/issues/3196
This PR adds the ability to select a range of items in list contexts. It
does this in two ways:
* Sticky range select: like what we already have in the staging view,
you press 'v' to toggle range select and then use up/down keys to extend
the range
* Non-sticky range select: rather than explicitly toggling this on/off,
you use shift+up/down to extend the range
The PR adds the ability to range select in all list contexts, but it's
up to individual actions to opt-in to supporting a range. This PR only
supports it for copying a range of commits for cherry-picking. We can
add more support iteratively so that we're not merging a single giant
PR. For all actions requiring selection of a single-item, an error will
be shown if a range is selected.
Other use cases we want to support in the near future:
* marking commits as pick/drop/fixup/squash/etc when mid-rebase
* fixup/squash/drop when outside rebase
* moving commits up/down (in or out of rebase)
* staging/unstaging multiple files
* discarding multiple files
## Updated keybindings
Because the 'v' binding is now globally dedicated to toggling range
select, I've changed the cherry-pick copy/paste keys from 'c' and 'v' to
'shift+C' and 'shift+V' respectively. I've also nullified the 'v'
keybinding on the 'view divergence from upstream' option in the upstream
options menu (conveniently it was the first option in the menu so you
can press enter on it).
## Standardised range select display
As a bonus, this PR standardises how we display a range select. We
already had range select support in the patch explorer view and merge
conflicts view, but they were directly rendering the highlighted
selection (i.e. blue background colour) in the content written to the
view, rather than tell the view which lines were selected and have the
view highlight them itself. A convenient benefit here is that now the
entire line is highlighted, including trailing space, rather than just
the content of the line. Another convenient benefit is that our
integration tests can now easily ask the view which lines are selected,
rather than depending on the specific context, because the view keeps
track of it.
I've removed the selectedRangeBgColor config option because
selectedLineBgColor should be fine. I don't see the need for two
options, but tell me if you think otherwise.
Also, another thing we're standardising on: hitting escape will cancel
the range select, which in the staging/patch-building views means if
you're selecting a range, you'll need to hit escape twice to exit out of
the view. For consistency, we're also applying this logic if you have a
hunk selected. I personally would much prefer this and have several
times accidentally exited out of the view when trying to cancel a range
select by pressing escape. In lazygit in general, 'escape' means 'exit
out of the innermost mode' and I would consider range select to be a
kind of mode.
## Sticky vs non-sticky range interaction
Here's the state machine that explains how the sticky and non-sticky
range select modes interact. Although users will typically pick one or
the other, it's important to be clear on what the logic is if you swap
between them:
```
(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
```
Also if you press escape in either range mode, it cancels the range
select.
## Some implementation details
* when the action involves toggling e.g. toggling cherry-pick copy or
toggling staged, we decide what to do based on the selection: for
example with staging: if there are any unstaged changes in the
selection, we'll stage everything, otherwise we unstage everything. This
is the logic we already had when staging individual directories.
* we retain range selection if a view loses focus
* where we previously set SetSelectedLineIdx all over the place (e.g.
setting selected line idx to 0 when checking out a branch) we're now
using SetSelection which also resets the range select. There are only a
couple of places where we would still want to use SetSelectedLineIdx
(e.g. when the user moves up/down a page, because they would want to
retain range select in that case)
- **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)
* [x] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [x] Docs (specifically `docs/Config.md`) have been updated if
necessary
* [ ] You've read through your own file changes for silly mistakes etc
<!--
Be sure to name your PR with an imperative e.g. 'Add worktrees view'
see https://github.com/jesseduffield/lazygit/releases/tag/v0.40.0 for
examples
-->
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
CI is erroneously saying that our integration tests are passing.
As @stefanhaller says in a comment:
> It broke with this commit:
aaecd6cc40.
Since that change, the run_integration_tests.sh script will exit with
the exit status of the the last mv command, not the test run. Should be
pretty easy to fix.
Thanks to Stefan for fixing this
(The reason I'm saying this all here in the PR description is that PR
descriptions now get included in merge commits)
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.
- **PR Description**
Addresses #3116.
I'm not 100% sure I like the behavior, but I put it out there so that
others can test it and form an opinion. It not only affects keybindings,
but also invoking menu items (either with enter or with their key
binding): the menu now stays open in that case, which I think is
actually better.
There's a horrible hack for keeping the integration tests working, I
don't have a good idea how to fix that for real. Suggestions welcome.
