Reproduction recipe:
1. stage all changes in a file by pressing space on it in the files panel
2. enter the staged changes panel by pressing enter
3. unstage one of the changes
This makes the unstaged changes panel visible, but keeps the focus in
the staged changes panel. However, the highlight in the unstaged changes
view becomes visible, as if it were focused.
Fixes#3664
An inactive selection is one where the view is part of the context stack, but
not the active view. For example, the files view when you enter the staging
panel, or any view when you open a panel.
Remove the old mechanism of clearing the highlight in Layout.
This fixes a problem with a wrong highlight showing up in the staging panel when
entering a file with only staged changes.
Reproduction recipe:
1. stage all changes in a file by pressing space on it in the files panel
2. enter the staged changes panel by pressing enter
3. unstage one of the changes
This makes the unstaged changes panel visible, but keeps the focus in the staged
changes panel. However, the highlight in the unstaged changes view becomes
visible, as if it were focused.
To explain why this happens, you need to know how the selection highlighting of
a view is turned on or off. It is turned on when it gains the focus, i.e. when
ActivateFocus is called on it, which in turn happens when PushContext is called.
It is turned off in Layout when gocui sees that the current view is no longer
the same as last time, in which case it calls onViewFocusLost on the previous
current view.
This mechanism only works reliably when there is at most one PushContext call
per event handler. If there is more than one, then the first one gets its
highlight turned on, then the second one, but since gocui has never seen the
first one as the active view in Layout, it doesn't get the highlight turned off
again even though it should.
And this happens in the above scenario. When pressing enter on a file with only
staged changes, we first push the staging context (in
FilesController.EnterFile), and then later we push the stagingSecondary context
when we realize we only have staged changes. This leaves the highlight of the
staging context on.
- **PR Description**
Fix a performance regression that I introduced with v0.41: when entering
or leaving staging mode for a file, or when switching from a file that
has only unstaged changes to one that has both staged and unstaged
changes, there was a noticeable lag of about 500ms on my machine. With
the improvements in this PR we get this back down to about 20ms.
runewidth.StringWidth is an expensive call, even if the input string is pure
ASCII. Improve this by providing a wrapper that short-circuits the call to len
if the input is ASCII.
Benchmark results show that for non-ASCII strings it makes no noticable
difference, but for ASCII strings it provides a more than 200x speedup.
BenchmarkStringWidthAsciiOriginal-10 718135 1637 ns/op
BenchmarkStringWidthAsciiOptimized-10 159197538 7.545 ns/op
BenchmarkStringWidthNonAsciiOriginal-10 486290 2391 ns/op
BenchmarkStringWidthNonAsciiOptimized-10 502286 2383 ns/op
In d5b4f7bb3e and 58a83b0862 we introduced a combined mechanism for rerendering
views when either their width changes (needed for the branches view which
truncates long branch names), or the screen mode (needed for those views that
display more information in half or full screen mode, e.g. the commits view).
This was a bad idea, because it unnecessarily rerenders too many views when just
their width changes, which causes a noticable lag. This is a problem, for
example, when selecting a file in the files panel that has only unstaged
changes, and then going to one that has both staged and unstaged changes; this
splits the main view, causing the side panels to become a bit narrower, and
rerendering all those views took almost 500ms on my machine. Another similar
example is entering or leaving staging mode.
Fix this by being more specific about which views need rerendering under what
conditions; this improves the time it takes to rerender in the above scenarios
from 450-500s down to about 20ms.
This reintroduces the code that was removed in 58a83b0862, but in a slightly
different way.
The rendering of remote branches is in no way dependent on the width of the view
(or the screen mode). Unlike in the local branches view, we don't truncate long
branch names here (because there's no more information after them).
This is an error introduced in d5b4f7bb3e.
- **PR Description**
Some of our menus let you pick a value for some option (e.g. the sort
order menus for branches and commits). It's nice to see which one is the
current value when opening such a menu, so this PR implements that.
For menus that also have key bindings, the radio button goes between the
key and the label.
As an alternative, I considered selecting the current value when the
menu is opened; however, we currently have no menus where we select a
different entry than the first one, and it might be confusing for people
who are used to opening a menu and then pressing down-arrow a certain
number of times to get to a particular value.
- **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 have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
For checkboxes it probably doesn't really make sense to use them yet, because
we'd have to find a way how you can toggle them without closing the dialog; but
we already provide rendering for them to lay the ground.
But radio buttons can be used already, because for those it is ok to close the
dialog when choosing a different option (as long as there is only one grounp of
radio buttons in the panel, that is).
Always show the "Discard unchanged changes" menu item in the Discard
menu, just strike it through if not applicable. This will hopefully help
with confusion about the meaning of "all" in the "Discard all changes"
entry; some people misunderstand this to mean all changes in the working
copy. Seeing the "Discard unstaged changes" item next to it hopefully
makes it clearer that "all" is meant in contrast to that.
Strike it through if not applicable. This will hopefully help with confusion
about the meaning of "all" in the "Discard all changes" entry; some people
misunderstand this to mean all changes in the working copy. Seeing the "Discard
unstaged changes" item next to it hopefully makes it clearer that "all" is meant
in contrast to that.
- **PR Description**
Several custom patch commands on parts of an added file would fail with the
confusing error message "error: new file XXX depends on old contents".
These were dropping the custom patch from the original commit, moving the
patch to a new commit, moving it to a later commit, or moving it to the index.
We fix this by converting the patch header from an added file to a diff against
an empty file. We do this not just for the purpose of applying the patch, but
also for rendering it and copying it to the clip board. I'm not sure it matters
much in these cases, but it does feel more correct for a filtered patch to be
presented this way.
Fixes#3679.
- **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 have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
Several custom patch commands on parts of an added file would fail with the
confusing error message "error: new file XXX depends on old contents". These
were dropping the custom patch from the original commit, moving the patch to a
new commit, moving it to a later commit, or moving it to the index.
We fix this by converting the patch header from an added file to a diff against
an empty file. We do this not just for the purpose of applying the patch, but
also for rendering it and copying it to the clip board. I'm not sure it matters
much in these cases, but it does feel more correct for a filtered patch to be
presented this way.
We're going to add another argument in the next commit, and that's getting a bit
much, especially when most of the arguments are bool and you only see true and
false at the call sites without knowing what they mean.
This currently works (albeit with a bit of manual work, as the user needs to
resolve conflicts), and we add this test just to make sure that we don't break
it with the following change.
This currently works, we add it as a regression test to make sure we don't break
it. It is an interesting test because it turns the deletion of the file in the
moved-from commit into a modification.
- **PR Description**
This fixes two layout problems with pagers that draw a horizontal line
across the entire width of the view (e.g. delta):
- sometimes the width of that line was one character too long or too
short in the staged changes view
- when changing from a file or directory that has only staged or only
unstaged changes to one that has both, the length of the horizontal line
was totally off and only redraw correctly at the next refresh
This is important when using a pager that draws a horizontal line across the
entire width of the view; when changing from a file or directory that has only
unstaged (or only staged) changes to one that has both, the main view is split
in half, but the PTY task would be run on the view in its old state, so the
horizonal line would be too long and wrap around.
All PTYs were created with the size of the main view, on the assumption that
main and secondary always have the same size. That's not true though; in
horizontal split mode, the width of the two views can differ by one because of
rounding, and when using a pager that draws a horizontal line across the width
of the view, this is visible and looks very ugly.
- **PR Description**
As a followup to [this
discussion](https://github.com/jesseduffield/lazygit/pull/3388#issuecomment-2002308045),
this PR adds a way to add a prompt text to menus. It is shown above the
menu items, separated by a blank line. We use it to add a prompt to the
remote branch checkout menu.
When switching to a repo that was open before, the context tree is reused, so
before adding keybinding functions to those contexts again, we need to clear the
old ones.
(Github decided to auto-close #2533, and I don't see any way to reopen
it, so opening a new one here. Please see there for discussion and
review.)
When pressing `>` in the commits panel to trigger loading all the
remaining commits past the initial 300, memory consumption is a pretty
big problem for larger repositories.
The two main causes seem to be
1. the cell memory from rendering the entire list of commits into the
gocui view
2. the pipe sets when git.log.showGraph is on
This PR addresses only the first of these problems, by not rendering the
entire view, but only the visible portion of it. Since we already
re-render the visible portion of the view on every layout call, this was
relatively easy to do.
Below are some measurements for our repository at work (261.985
commits):
| | master | this PR |
| ------------- | ------ | ------- |
| without Graph | 855 MB | 360 MB |
| with Graph | 3.1 GB | 770 MB |
And for the linux kernel repo (1.170.387 commits):
| | master | this PR |
| ------------- | ----------------------------------------- | ------- |
| without Graph | 5.8 GB | 1.2G |
| with Graph | Killed by the OS after it reached 86.9 GB | 39.9 GB |
The measurements were taken after scrolling all the way down in the list
of commits. They have to be taken with a grain of salt, as memory
consumption fluctuates quite a bit in ways that I find hard to make
sense of.
As you can see, there's more work to do to reduce the memory usage for
the graph, but for our repo at work this PR makes it usable already,
which it wasn't really before.
When refreshViewportOnChange is true, we would refresh the viewport once at the
end of FocusLine, and then we would check at the end of AfterLayout if the
origin has changed, and refresh again if so. That's unnecessarily complicated,
let's just unconditionally refresh at the end of AfterLayout only.
We want to add an additional method to ISearchableContext later in this branch,
and this will make sure that we don't forget to implement it in any concrete
context.
Searching in the "Divergence from upstream" view would select the wrong lines.
The OnSearchSelect function gets passed a view index, and uses it to select a
model line. In most views these are the same, but not in the divergence view
(because of the Remote/Local section headers).
ListContextTrait.OnSearchSelect was introduced in 138be04e65, but it was never
called. I can only guess that a planned refactoring wasn't finished here.
