- **PR Description**
Improve the dispatching of key bindings so that remapping "pullFiles" to
a different key works correctly in the Commits panel.
Fixes#4614.
Previously we would call pullFiles() from the pick() handler if we were not in a
rebase, assuming that the default keybinding for both is "p". This needn't be
the case of course, if the user has remapped one or the other.
The consequence of this was that swapping the keybindings for "pullFiles" and
"pushFiles" would work in all panels except the Commits panel (unless "pick" was
also remapped in the same way).
Fix this by using the new AllowFurtherDispatching mechanism of DisabledReasons
to pass the keybinding on to the next handler.
If a DisabledReason has its AllowFurtherDispatching flag set, it is returned as
a ErrKeybindingNotHandled error, instead of shown as a toast right away. This
allows gocui to continue to dispatch the keybinding, and we can unwrap the error
at the other end (in our global ErrorHandler) and display it then.
This allows having keybindings for the same key at the local and global levels,
and they will continue to be dispatched even if the first one returns a
DisabledReason. It is opt-in, so we only use it for cases where we know that a
local and a global handler share the same (default) keybinding.
There was no reason to declare a variable for disabledReason, assign it inside
the "if binding.GetDisabledReason != nil" statement, and then check its value
again after that if statement. Move all that code inside the first if statement
to make the control flow easier to understand.
Adaptions are for this gocui commit:
Cleanup: remove Is* error functions
- Use errors.Is instead of quality comparisons. This is better because it
matches wrapped errors as well, which we will need later in this branch.
- Inline the errors.Is calls at the call sites. This is idiomatic go, we don't
need helper functions for this.
See https://go.dev/blog/go1.13-errors for more about this.
- **PR Description**
When switching between repos, each repo might have a different focused
panel; in this case, the previously focused panel would show the
"inactive" highlight. By default this is only bold text, so it's barely
noticeable, but it becomes more pronounced when setting e.g.
```yml
gui:
theme:
inactiveViewSelectedLineBgColor:
- "#666666"
```
I noticed this especially when entering or leaving submodules; for
example, enter a submodule by pressing enter in the Files panel, then
switch to the Commits panel in the submodule, then press Esc to go back
to the parent repo. This would put the focus back into the Files panel,
but keep the inactive highlight in the Commits panel.
When switching between repos, each repo might have a different focused panel; in
this case, the previously focused panel would show the "inactive" highlight. By
default this is only bold text, so it's barely noticeable, but it becomes more
pronounced when setting e.g.
gui:
theme:
inactiveViewSelectedLineBgColor:
- "#666666"
I noticed this especially when entering or leaving submodules; for example,
enter a submodule by pressing enter in the Files panel, then switch to the
Commits panel in the submodule, then press Esc to go back to the parent repo.
This would put the focus back into the Files panel, but keep the inactive
highlight in the Commits panel.
- **PR Description**
Allows the reset menu to have a different name that is displayed, and a
fully qualified name that git will unambiguously know what it refers
about. We could totally squash this back down to 1 input, and display to
the user the _precise_ full ref name that we are resetting to, but I
think the context they are in (branches tab versus tag tab), means that
we don't need to do that, and can continue to just show the branch name
and the tag name to the end users.
Fixes https://github.com/jesseduffield/lazygit/issues/4569
In the unlikely scenario that you have a remote branch on `origin` called
`foo`, and a local tag called `origin/foo`, git changes the behavior of
the previous command such that it produces
```
$ git for-each-ref --sort=refname --format=%(refname:short) refs/remotes
origin/branch1
remotes/origin/foo
```
with `remotes/` prepended. Presumably this is to disambiguate it from
the local tag `origin/foo`. Unfortunately, this breaks the existing
behavior of this function, so the remote branch is never shown.
By changing the command, we now get
```
$ git for-each-ref --sort=refname --format=%(refname) refs/remotes
refs/remotes/origin/branch1
refs/remotes/origin/foo
```
This allows easy parsing based on the `/`, and none of the code outside
this function has to change.
----
We previously were not showing remote HEADs for modern git versions
based on how they were formatted from "%(refname:short)".
We have decided that this is a feature, not a bug, so we are building
that into the code here.
- **PR Description**
When pressing N to move new commits to a new branch we get greeted with
an empty prompt, this PR makes it so we fill the empty prompt with a
suggestion taken from branchPrefix, similar to the good old create a new
branch.
Moving the getter of the suggested branch name to a separate function
allows us to reuse it in situations where we are not calling the regular
create new branch function, such as move commits to a new branch
Signed-off-by: Elias Assaf <elyas51000@gmail.com>
- **PR Description**
When refreshing the branches list, we have code to keep the same branch
selected even when the refresh changes the sort order; this code
remembers the selected branch before the refresh, and then tries to
select it again afterwards (looking it up by name) if it is still there.
However, we stored the previously selected branch too early, before even
obtaining the branches list; if the user moved the selection between
that point and the end of the refresh, it would jump back. Fix this by
remembering the previous selection only at the last moment, right before
assigning the new branches slice.
We still have a race condition here between the UI code that manages the
selection as the user presses arrow keys, and the background thread
doing the refresh that reads and restores the selection; however, the
race was there before, and we make it neither better nor worse with this
PR. It doesn't seem to be a problem in practice.
Fixes#4116.
When refreshing the branches list, we have code to keep the same branch selected
even when the refresh changes the sort order; this code remembers the selected
branch before the refresh, and then tries to select it again afterwards (looking
it up by name) if it is still there.
However, we stored the previously selected branch too early, before even
obtaining the branches list; if the user moved the selection between that point
and the end of the refresh, it would jump back. Fix this by remembering the
previous selection only at the last moment, right before assigning the new
branches slice.
We still have a race condition here between the UI code that manages the
selection as the user presses arrow keys, and the background thread doing the
refresh that reads and restores the selection; however, the race was there
before, and we make it neither better nor worse with this PR. It doesn't seem to
be a problem in practice.
Previously we would enter a newline at the password prompt, which would
cause the fetch to fail. The problem with this was that if you have many
remotes, the fetch would sometimes hang for some reason; I don't totally
understand how that happened, but I guess the many ssh processes
requesting passwords would somehow interfere with each other. Avoid this
by simply killing the git fetch process the moment it requests the first
password.
Previously we would enter a newline at the password prompt, which would cause
the fetch to fail. The problem with this was that if you have many remotes, the
fetch would sometimes hang for some reason; I don't totally understand how that
happened, but I guess the many ssh processes requesting passwords would somehow
interfere with each other. Avoid this by simply killing the git fetch process
the moment it requests the first password.
- **PR Description**
In #4346 we added a `/` root item in the Files and CommitFiles panels
whenever there is more than one top-level item. We made it
unconditional, but I promised to add a config as soon as users ask for
being able to disable it. For a while I was able to convince users who
asked for it that it is useful and they don't want to turn it off, but
now there's a [stronger
request](https://github.com/jesseduffield/lazygit/discussions/4590#discussioncomment-13254924)
from someone who refuses to upgrade to the current version, and we don't
want that.
So, add a config option `gui.showRootItemInFileTree` that is true by
default.
We will need a user config in the file tree in the next commit, and passing the
entire common is the easiest way to do that while ensuring hot-reloading when
users change the config while lazygit is running.
Fix a regression introduced with #4525.
This fixes the problem that background fetching makes lazygit hang when
the fetch request needs to prompt for a passphrase. For Mac users who
use the keychain to store their ssh passphrases, this can happen when
lazygit is running while the machine goes to sleep, because macOS looks
the keychain in that case.
This is a regression introduced with a199ed1396c; it is important to use a PTY
even with credentialStrategy=FAIL, otherwise the fetch command will spew the
credentials request into the UI and then hang.
This fixes the problem that background fetching makes lazygit hang when the
fetch request needs to prompt for a passphrase. For Mac users who use the
keychain to store their ssh passphrases, this can happen when lazygit is running
while the machine goes to sleep, because macOS looks the keychain in that case.
- **PR Description**
The status view is not supposed to be focusable right now. (This might
change soon, but for now it isn't.) Pressing '0' on it does nothing.
However, clicking on it would still focus it, because the click handler
in MainViewController assumed that when the clicked view doesn't have
the focus, then its "other" view must have, and we just want to toggle
the focus between the two (like when pressing tab). It didn't take the
possibility into account that the current side panel isn't focusable at
all; if it was, then its SwitchToFocusedMainViewController would have
handled the click.
To fix this, check if the "other" view has the focus before handling the
click, and do nothing otherwise.
This also fixes clicking in the main views of the Worktrees or
Submodules tabs, or any other tabs whose main views are not focusable.
Fixes#4566.
The click handler of MainViewController was registered as a global handler, so
it was used when a side panel was focused that doesn't have a
SwitchToFocusedMainViewController attached (e.g. Status, Worktrees, or
Submodules). This handler would then push the main view context, but with the
code that is meant only for toggling between the main view pair contexts, i.e.
with taking over the parentContext from the otherContext, which doesn't have one
at that point. This would later lead to a crash in onClick because the
parentContext was nil.
Fix this by splitting the click handler in two, one for when it already has the
focus, and one for toggling from the other view, and make these focus specific.
- **PR Description**
In #4404 we added home/end support to confirmation popups. This broke
handling of the home/end keys in prompts, where they used to work as
alternatives to ctrl-a and ctrl-e.
Here's one way (maybe not the best) to fix this.
A better fix might have been to have separate views for confirmations
and prompts, so that we can have different keybindings for each. That's
a bit more work though.
Fixes#4553.
Ok, this is a long one. (It took me all weekend to figure out.)
We seem to have a race condition between re-rendering the main view and
the [layout
code](a0ec22c251/pkg/gui/layout.go (L75))
that checks whether a view has become smaller and therefore needs to
scroll up. When rerendering the main view, we are careful not to
invalidate the ViewLines array, so the code first calls Reset on the
view, and then starts writing lines to the view until we have written
enough for its old scroll position, and only then do we trigger a
redraw. This is all well and good, but theoretically it could happen
that the above-mentioned layout code runs shortly after the task has
started writing lines to the view (say, after the first one has been
written), and then it would see that the view's height is only 1, and
scroll it all the way to the top.
I have never seen this happen, so it seems that we are being lucky and
the race condition is only a theoretical one.
However: we had a very silly and embarrassing bug in the
focused-main-view code that triggers the race condition occasionally.
The bug is that when the main view is focused, we would refresh it
multiple times in quick succession, once for every side panel that is
being refreshed (instead of just once for the side panel that it belongs
to). So the first task would call Reset, start writing lines to the
view, and then the second task would come along, kill the first, call
Reset again, and start writing lines again, and so on. Apparently this
made it more likely for the layout code to run concurrently with this,
and see the view at a moment where it only has one or two lines. I have
seen it scroll to the top on its own a few times, which is very annoying
when you are in the middle of reviewing a longer commit, for instance.
We can fix this by refreshing the main view only for the side panel that
it belongs to, which is what this PR does. I have let lazygit run over
night with a `refresher.refreshInterval` value of 3, and it hadn't
scrolled to the top when I came to look in the morning, which makes me
pretty confident that we're good now.
It would still be nice if we could fix the race condition for real too,
but it's less urgent now, and it also doesn't seem trivial. I guess
instead of writing lines directly to the view we would have to buffer
them first, and only write them to the view when the original scroll
position is reached (with some synchronization, e.g. with a OnUIThread).
There are other complications that make this tricky though, and I have
no plans right now to tackle this.
Previously we would re-render the focused main view several times during a
refresh, once for every side panel. While this should usually not be noticeable
for users because we are careful to avoid flicker when refreshing the main view,
this would sometimes lead to the main view scrolling up to the top by itself;
see PR description for more details.
- **PR Description**
This might be useful to see in general (users will normally only see it
after they quit lazygit again, but still). But it is especially useful
when writing back the config file fails; some users have their config
file in a read-only location, so we had reports of lazygit no longer
starting up when migration was necessary. #4210 was supposed to improve
this a bit, but it didn't tell users what changes need to be made to the
config file. Now we tell them, and users can then make these changes
manually if they want.
We do this only at startup, when the GUI hasn't started yet. This is
probably good enough, because it is much less likely that writing back a
migrated repo-local config fails because it is not writeable.
Example output:
```
The user config file /Users/stk/Library/Application Support/lazygit/config.yml must be migrated. Attempting to do this automatically.
The following changes were made:
- Renamed 'gui.windowSize' to 'screenMode'
- Changed 'null' to '<disabled>' for keybinding 'keybinding.universal.confirmInEditor'
- Changed 'stream: true' to 'output: log' in custom command
Config file saved successfully to /Users/stk/Library/Application Support/lazygit/config.yml
```
The branch also contains a lot of code cleanups.
- **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
This is for the unlikely case that a repo-local config file can't be written
back after migration; in this case we can't log the migration changes to the
console, so include them in the error popup instead.
This might be useful to see in general (users will normally only see it after
they quit lazygit again, but still). But it is especially useful when writing
back the config file fails for some reason, because users can then make these
changes manually if they want.
We do this only at startup, when the GUI hasn't started yet. This is probably
good enough, because it is much less likely that writing back a migrated
repo-local config fails because it is not writeable.
Most migrations happen at startup when loading the global config file, at a time
where the GUI hasn't been initialized yet. We can safely print to the console at
that point. However, it is also possible that repo-local config files need to be
migrated, and this happens when the GUI has already started, at which point we
had better not print anything to stdout; this totally messes up the UI.
In this commit we simply suppress the logging when the GUI is running already.
This is probably good enough, because the logging is mostly useful in the case
that writing back the migrated config file fails, so that users understand
better why lazygit doesn't start up; and this is very unlikely to happen for
repo-local config files, because why would users make them read-only.
