- **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.
It's a bit silly to find out by string comparison whether computeMigratedConfig
did something, when it knows this already and can just return the information.
This doesn't make a huge difference to the production code; the string
comparison isn't very expensive, so this isn't a big deal. However, it makes the
tests clearer; we don't have to bother specifying an expected output string if
the didChange flag is false, and in particular we can get rid of the ugly "This
test intentionally uses non-standard indentation" bit in one of the tests.
- **PR Description**
Fix crash in AutoForwardBranches when the branches slice is empty.
I'm not aware of any real scenario where this can happen, but we have
seen one stack trace where it crashed with an out-of-bounds error in the
range expression below, so there must be a way. And it seems better to
guard against it anyway, rather than assuming it can't happen.
(Hopefully) fixes#4564.
I'm not aware of any real scenario where this can happen, but we have seen one
stack trace where it crashed with an out-of-bounds error in the range expression
below, so there must be a way. And it seems better to guard against it anyway,
rather than assuming it can't happen.
- **PR Description**
This is a recreation of
https://github.com/jesseduffield/lazygit/pull/4556.
Compared to that PR description, this has actually solved the problem
that repeated attempts to `1` `a` when `gui.statusPanelView: dashboard`.
Now we only rotate logs on the 2nd `a` press. While this does differ
from current behavior, I believe it is better, and I have more details
in my commit description.
Fixes https://github.com/jesseduffield/lazygit/issues/4469
This now allows for leaving the status panel and returning back to the
same log command. Previously any return to the status panel would result
in the next command in the list being shown. Now, you need to press `a`,
with a log command being rendered, to rotate to the next
allBranchesLogCmd.
- **PR Description**
Fixes the problem described in #4557:
I often find myself initiating a commit message with `c`, but then
halfway through typing the message, realize that is is likely to be more
substantial than I thought. I press `<esc>` and then `C` to launch my
editor where I can edit a larger commit body more effectively. I finish
my commit message, close my editor, return to LazyGit and all is well.
Unfortunately, the next time I press `c`, creating a totally new commit,
my partially completed message is still present. It no longer is
relevant, so I have to delete the few words I typed before.
In one case it was actually called *before* making a commit (when switching from
the commit message panel to committing in the editor). And clearing the
preserved message is the only thing it does, so name it after what it does
rather than when it's called.
Previously it would only clear the message if the panel had been opened with
preserveMessage=true; we don't need this check, because callers know how they
opened the panel, and whether they want to clear the message.
This is no change in behavior because OnCommitSuccess only clears the message
when the commit message panel was opened with preserveMessage=true, which it
isn't in the case of creating a tag.
And this is in fact the desired behavior, because we don't want creating a tag
to interfere with preserving commit messages in any way.
- **PR Description**
The default binding for ConfirmInEditor is <a-enter>, which has two
problems:
- some terminal emulators don't support it, including the default
terminal on Mac (Terminal.app)
- on Windows it is bound to toggling full-screen
Ideally we would use `<c-enter>` instead (and Command-Enter on Mac), but
neither is possible without https://github.com/gdamore/tcell/issues/671,
so for the time being add an alternate keybinding which works
everywhere.
Show both bindings in the footer of the commit description panel if they
are both non-null. While we're at it, fix the footer for the case where
either or both of the keybindings are set to `<disabled>`.
And finally, change "commit" to "submit" in that footer; we use the same
panel also for creating tags, in which case "commit" is not quite right.
The default binding for ConfirmInEditor is <a-enter>, which has two problems:
- some terminal emulators don't support it, including the default terminal on
Mac (Terminal.app)
- on Windows it is bound to toggling full-screen
Ideally we would use <c-enter> instead (and Command-Enter on Mac), but neither
is possible without https://github.com/gdamore/tcell/issues/671, so for the time
being add an alternate keybinding which works everywhere.
Show both bindings in the footer of the commit description panel if they are
both non-null. While we're at it, fix the footer for the case where either or
both of the keybindings are set to <disabled>.
And finally, change "commit" to "submit" in that footer; we use the same panel
also for creating tags, in which case "commit" is not quite right.
- **PR Description**
In longer branches there's often the need to jump to the beginning of
the branch, e.g. in order to re-review all commits from the beginning.
There's no easy way to do this in lazygit.
In this PR I overload the "go to bottom" key (`>`) to jump to the first
commit of the current branch if the selection is above it, and only then
jump to the very bottom. I like that we don't need to introduce a new
key binding for this.
It's never called, the binding ListController.HandleGotoBottom wins.
The functionality of loading more commits is implemented by GetOnFocus, and this
way it works not only for '>', but also for other navigation keys like page
down.
BeginInteractiveRebaseForCommit is used for all the patch commands, and for
rewording. It works by setting the commit we want to stop at to 'edit'; this
doesn't work for merge commits. This wasn't a problem for the patch commands so
far, because you typically don't use custom patches with merge commits (although
we don't prevent this; maybe we should?).
However, it was a problem when you tried to reword a merge commit; this
previously failed with an error, as the test added in the previous commit
demonstrated.
Also, we want to add a new patch command that has to stop *before* the selected
commit (pull patch to new commit before the original one), and this wouldn't
work for the first commit in a feature branch, because it would have to set the
last commit before that to 'edit', which isn't possible if that's a merge (which
is likely).
To fix all this, use a 'break' before the selected commit if the commit is a
merge. It is important that we only do it in that case and not always, otherwise
we would break the new regression tests that were added a few commits ago.
I almost broke this during the development of this branch, so add a test to
guard against that. The point here is that the stack remains intact, i.e. the
newly created commit is the last commit of the lower branch, and thus shows the
"*".
I almost broke this during the development of this branch, so add a test to
guard against that. The point here is that the stack remains intact, i.e. the
renamed commit is the head of the lower branch, and thus shows the "*".
