- **PR Description**
As of #4684, hunk mode has become so useful that I prefer it over line
mode now. This PR adds a config that lets you use hunk mode by default
in the staging view.
I'm not enabling this by default yet, although I do think it's the more
useful mode for most people. The biggest issue that I still have with
this is that _if_ you need to switch to line mode for some reason, then
it's very non-obvious how to do that. New users might not find out at
all, and think that lazygit doesn't allow staging individual lines.
- **PR Description**
Hunk selection mode is one of the features that many people don't know
about, because it is not very discoverable. You can switch to it from
line selection mode by pressing `a` in the staging view.
The problem with this mode is that it selects entire hunks, where hunks
are defined to be sections of the diff starting with `@@`. Very often,
hunks consist of multiple distinct blocks of changes, separated by
context lines. For example, with the default diff context size of 3 it
takes at least 6 unchanged lines between blocks of changes for them to
be separated into distinct hunks; if there are 5 or less unchanged lines
between them, they are grouped into one hunk. And of course, if you
increase the diff context size by pressing `}`, you will get even fewer
hunks.
Now, most of the time I want to navigate between the individual blocks
of changes in a diff, regardless of how git groups them into hunks.
That's what this PR does: when pressing `a`, the selection is extended
to just the current group of changes, separated by context lines; you
can easily stage it by pressing space, and the selection will move on to
the next block of changes. Actual hunks no longer play a role here.
Also, in line selection mode the right/left arrow keys now move between
blocks of changes rather than actual hunks.
I find this new behavior so useful that I almost always switch to hunk
mode right away after entering the staging view. It saves a lot of
keystrokes, since it is very rare that I want to select only some lines
of a block of adjacent changes. This makes me wonder whether we should
enable hunk mode by default when entering staging, but that's going to
be another PR.
It's annoyingly slow, and there is a separate task for generating the list if
needed.
Also, clear the terminal before running the test; this makes it easier to see
the results of the last test run.
We only want to do this when the function is called from the remote branches
panel. It can also be called with a selection of local branches in order to
delete their remote branches, but in this case the selection shouldn't be
collapsed because the local branches stay around.
We had code already that was supposed to do this, but it didn't work. It should
have used SetSelection() instead of SetSelectedLineIdx(); the latter doesn't
actually cancel a range selection.
Introduce a new function specifically for collapsing the range after deleting
multiple items, so that clients don't need two calls (we'll add a bunch more in
this branch).
- **PR Description**
While it's true that the behavior is a little different from the staging
panel, where the staged lines are actually removed from the view and in
many cases the selection stays more or less in the same place, it is
still very useful to move to the next stageable thing in the custom
patch building view too.
Also, we change the visualization of what's included in the patch to
mark only the + and - lines of the patch; for all other lines it doesn't
make a difference whether they are included.
And finally, we make it so that only + and - lines are considered when
pressing space; previously it would also look at selected context lines,
which doesn't make much sense. This improves the experience for mouse
users who like to generously select hunks by dragging across them,
including some context lines above and below.
While it's true that the behavior is a little different from the staging panel,
where the staged lines are actually removed from the view and in many cases the
selection stays more or less in the same place, it is still very useful to move
to the next stageable thing in the custom patch building view too.
This improves the experience when selecting a hunk generously with the mouse, by
dragging over it including some context lines above and below. Previously we
would consider the "moving end" of the selection range for whether things need
to be added or removed, but this doesn't make sense if it's a context line. Now
we consider the first actual change line that is included in the range.
It is confusing to get header lines, hunk headers, or context lines rendered as
being included in a custom patch, when including these makes no difference to
the patch.
This is only a visual change; internally, we still record these non-patch lines
as being included in the patch. It doesn't matter though; you can press space on
a header line and nothing happens.
It would probably be cleaner to only record + and - lines in the includedLines
array, but that would be a bit more work, and doesn't seem worth it.
- **PR Description**
For cherry-picking, this used to work in earlier versions, but it broke
in #4443. For reverting, it was never supported.
Also, we add some minor improvements while we're at it, such as slightly
better names for the auto-stashes that are created for the various
operations (so that, if an auto-stash pop fails and the stash is kept
around, you can tell more easily what it was for). Also, we now adjust
the selection of the commits list after cherry-picking, so that the same
commit stays selected.
CheckMergeOrRebase calls Refresh already. However, it does an async refresh by
default, so we must turn this into a sync refresh so that moving the selection
down by one works even for the very first commit in history. Also, we must add
an explicit call to FocusLine so that the view selection is in sync with the
model selection; previously this was taken care of by the PostRefreshUpdate call
that happens as part of a refresh.
Keep the same commit selected, by moving the selection down by the number of
cherry-picked commits. We also do this when reverting commits, and it is
possible now that we use a sync waiting status.
We also need to turn the refresh that happens as part of CheckMergeOrRebase into
a sync one, so that the commits list is up to date and the new selection isn't
clamped.
We are about to change the selection behavior when cherry-picking, and it's good
to have tests that document in what way it changes in the next commit.
For the case of creating a new branch by moving commits to it, we were using the
current (old) branch name in the stash name; change this to use the new name
instead.
It seems to be excluded already when you run the lint.sh script, but in VS Code
when setting Lint on Save to package it would still lint a file in gocui when
you save it, which is annoying. (Remove the other paths that were there before;
they seem to be unused, and they were added by the auto-migration.)
Unfortunately, gopls will still lint gocui files with its builtin staticcheck
linter, and I couldn't find a way to turn this off. This might be a reason to
turn off staticcheck in gopls (not sure yet).
- **PR Description**
Applying or reverting a custom patch when one of the files contained in
the patch had unstaged modifications in the working tree would fail with
the confusing error message "does not match index", regardless of
whether those modifications conflicted with the patch or not. You would
expect this to just work if there are no conflicts, or to get the usual
conflict markers if there are. It was possible to work around this by
manually staging those files, but few people knew about this. Fix this
by staging the files (after asking for confirmation).
Also, fix a minor problem where an auto-stash for the "move patch to
index" command was forgotten to be dropped.
Unlike moving a patch to the index, applying or reverting a patch didn't
auto-stash, which means that applying a patch when there's a modified (but
unstaged) file in the working tree would error out with the message "error:
file1: does not match index", regardless of whether those modifications conflict
with the patch or not.
To fix this, we *could* add auto-stashing like we do for the "move patch to
index" command. However, in this case we rather simply stage the affected files
(after asking for confirmation). This has a few advantages:
- it only changes the staging state of those files that are contained in the
patch (whereas auto-stashing always changes all files to unstaged)
- it doesn't unnecessarily show a confirmation if none of the modified files are
affected by the patch
- if the patch conflicts with the modified files, the conflicts were "backwards"
("ours" was the patch, "theirs" the modified file); it is more logical if "ours"
is the current state of the file, and "theirs" is the patch.
It's a little unfortunate that the behavior isn't exactly the same as for "move
patch to index", but for that one we do need the auto-stash because of the
rebase that runs behind the scenes.
The tests show that this currently fails with the confusing error message "does
not match index", regardless of whether the patch conflicts with the
modifications or not. We'll improve this in the next commit.
I don't bother adding tests for reverting a patch, as the code is basically the
same as for apply.
This is functionality that works already, we only add the test for more complete
test coverage. However, there's a detail problem, and the test demonstrates
this: we keep the stash even if there was no conflict. We'll fix this next.
- **PR Description**
In the Stash panel, show information about the selected stash above the
diff of the stash in the main view. This is useful for stash entries
with very long names that are not fully visible in the Stash panel.
Fixes#4662.
Refresh is one of those functions that shouldn't require error handling (similar
to triggering a redraw of the UI, see
https://github.com/jesseduffield/lazygit/issues/3887).
As far as I see, the only reason why Refresh can currently return an error is
that the Then function returns one. The actual refresh errors, e.g. from the git
calls that are made to fetch data, are already logged and swallowed. Most of the
Then functions do only UI stuff such as selecting a list item, and always return
nil; there's only one that can return an error (updating the rebase todo file in
LocalCommitsController.startInteractiveRebaseWithEdit); it's not a critical
error if this fails, it is only used for setting rebase todo items to "edit"
when you start an interactive rebase by pressing 'e' on a range selection of
commits. We simply log this error instead of returning it.
This was added after this PR comment:
https://github.com/jesseduffield/lazygit/pull/3276#discussion_r1469077611
> Can we do a refresh after this reset so that the screen shows that the patch
> has been cancelled? That way, if we cancel on the next popup, the screen will
> be in a valid state.
I don't understand what "cancel on the next popup" means; there is no further
popup after this code.
- **PR Description**
This will format the `z` keyboard shortcut reference in this text in the
same way as all other keyboard shortcuts in the README.md file.
