- **PR Description**
In the solving of #4194 we had some discussion about the repeated
marshalling and unmarshalling of the content in our migrations.
https://github.com/jesseduffield/lazygit/issues/4194#issuecomment-2661375136.
I was feeling the itch to see if there was a meaningful performance
impact to that, and this is the results! I added a benchmark first that
uses the entirety of `Config.md`. This ensures a worst-case scenario for
the repeated parsing, and probably at least 1 user has done this for
their config. Here were the benchmark results prior and after this PR:
## Before
```
$ go test ./pkg/config/ -bench=. -benchmem -count=10
goos: linux
goarch: amd64
pkg: github.com/jesseduffield/lazygit/pkg/config
cpu: Intel(R) Core(TM) i7-8705G CPU @ 3.10GHz
BenchmarkMigrationOnLargeConfiguration-8 85 18165916 ns/op 1501442 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 50 21024442 ns/op 1501473 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 75 18814769 ns/op 1501509 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 60 17886812 ns/op 1501434 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 97 17767498 ns/op 1501448 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 62 18749923 ns/op 1501478 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 62 19248265 ns/op 1501467 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 92 14771199 ns/op 1501423 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 67 16345152 ns/op 1501440 B/op 19316 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 98 16785737 ns/op 1501472 B/op 19316 allocs/op
PASS
ok github.com/jesseduffield/lazygit/pkg/config 14.474s
```
so somewhere in the range of 14 to 21 ms added to startup time.
## After
```
$ go test ./pkg/config/ -bench=. -benchmem -count=10
goos: linux
goarch: amd64
pkg: github.com/jesseduffield/lazygit/pkg/config
cpu: Intel(R) Core(TM) i7-8705G CPU @ 3.10GHz
BenchmarkMigrationOnLargeConfiguration-8 440 2936145 ns/op 265800 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 428 3099183 ns/op 265787 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 355 3074069 ns/op 265785 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 478 3031144 ns/op 265792 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 375 2795470 ns/op 265785 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 406 2688491 ns/op 265791 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 375 3092990 ns/op 265791 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 379 2789022 ns/op 265785 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 417 3108530 ns/op 265785 B/op 3928 allocs/op
BenchmarkMigrationOnLargeConfiguration-8 487 2848502 ns/op 265788 B/op 3928 allocs/op
PASS
ok github.com/jesseduffield/lazygit/pkg/config 15.352s
```
So somewhere between 2.6 and 3.1 ms.
Is that a meaningful speedup? I'm not sure, but it's something! I had to
add the bit of complexity mentioned in the comment to track whether we
had made any changes, but that doesn't feel fundamentally much more
complex to me, since we were already implicitly tracking that fact with
whether the string file changed.
- **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)
* [ ] 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
- **PR Description**
Filter out [dev] comments earlier. Previously we only filtered them out
from the example config section in Config.md, but they still appeared in
the schema. This is not ideal, because the schema descriptions can
appear in editors on mouse hover or in auto-completions. So filter them
out earlier, so that they don't appear in the schema either.
Previously we only filtered them out from the example config section in
Config.md, but they still appeared in the schema. This is not ideal, because the
schema descriptions can appear in editors on mouse hover or in auto-completions.
So filter them out earlier, so that they don't appear in the schema either.
- **PR Description**
Some people have post-checkout hooks that take a lot of time, which
makes discarding changes slow. You can argue that a post-checkout hook
should only run when you switch branches, so it doesnt't have to run
when checking out single files or directories. You can also argue that
lazygit might have implemented discarding changes by taking the current
patch and applying it in reverse, which wouldn't have run a
post-checkout hook either.
So disable them for all cases where we use git checkout with a path;
this includes checking out a file from the commit files view.
Fixes#4272.
Some people have post-checkout hooks that take a lot of time, which makes
discarding changes slow. You can argue that a post-checkout hook should only run
when you switch branches, so it doesnt't have to run when checking out single
files or directories. You can also argue that lazygit might have implemented
discarding changes by taking the current patch and applying it in reverse, which
wouldn't have run a post-checkout hook either.
So disable them for all cases where we use git checkout with a path; this
includes checking out a file from the commit files view.
- **PR Description**
The current implementation of calculating sidePanelWidths does not
support any number higher than 0.5. Past that point, `mainSectionWidth`
will always be 0 because `1/0.6` = 1.6666 which gets truncated to 1,
which minus 1 is 0.
This PR proposes an alternative way, which effectively just splits the
horizontal range into 24 boxes, and the range from 0 to 1 dictates what
percentage of the boxes they get. I think this matches what the docs
have always claimed, which is:
```
Fraction of the total screen width to use for the left side section.
```
The number 24 was chosen intentionally so that the default users of
0.33333 will not see any changes in their behavior.
Users of the primary numbers 0.2, 0.15, and 0.1 will still retain their
ratios too! (by sheer coincidence).
There is one technical thing that I do not understand. On the first
implementation of this, I chose to make the ratio 1000, which broke the
entire thing. The outputs were not evenly distributed at all, with a
tiny jump from 0.7 to 0.8, but a huge jump to 0.9.
> Note: While writing up this PR, I tried to re-test this and I couldn't
reproduce... I'm leaving this in here just because it was an oddity. And
looking at the downstream `normalizeWeights` function, there clearly is
some work to find the lowest common factor, which would get trickier
when comparing 567 and 433. Is doing the computations on the Weight 24
something we should worry about for some reason?
Fixes https://github.com/jesseduffield/lazygit/issues/3721
- **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
1) the cron schedule was wrong: it was doing every saturday, rather than
the first saturday of each month.
2) It wasn't triggering a deploy despite pushing a tag because clearly
github doesn't want that to happen.
Now it triggers a deploy, and it also allows triggering from the UI,
letting you specify minor/patch bump and whether to ignore blocking
PRs/issues. As such I'm removing support for the old method of pushing
the tag. The new way is the only way.
- **PR Description**
The issue statement https://github.com/jesseduffield/lazygit/issues/4310
is exactly right, that the `commitPrefixes` element improperly claims
that it has modified the yaml whenever it exists, even if it does not
need to do changes.
Now, we initialize it to false, only set it to true inside our
modification section of the for loop.
Tests updated to add one that would have failed prior to this change.
The syntax change to use named struct fields instead of positional
fields felt nice since I wanted to just define on a single one of them
the `assertAsString` field.
The reason that field is required at all is the 2nd complaint on the
linked issue about the formatting change, is I don't believe is
something that is trivial to fix. I observed on existing migrations
before I wrote this one. But if it is easy to wrap up into this, let me
know!
Also, how do we normally backport things into previous releases? We'll
probably want this to make it into a `0.47.2`.
- **Please check if the PR fulfills these requirements**
* [ ] 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
- **PR Description**
Use indentation of 2 when rewriting auto-migrated config file. This
seems to be what most people use when indenting yaml files, and it seems
to make more sense than the default of 4. We also use it the example
config in Config.md.
This seems to be what most people use when indenting yaml files, and it seems to
make more sense than the default of 4.
We also use it the example config in Config.md.
- **PR Description**
Adds a guard around all global keybinds except for quitting the
application when a popup window is active. Users must now confirm, or
cancel, the popup prior to taking other action. This fixes
https://github.com/jesseduffield/lazygit/issues/4052, and will also
prevent other such confusing cases where popups are created, but never
removed.
In ff4ae4a544 we changed the order of the calls to render before
selecting the branch. This was done only to save an extra call to
ReApplyFilter, which is done by refreshView; I claimed that the order of
refreshView vs. SetSelectedLineIdx doesn't matter here. I guess I was
wrong about that, it makes the integration test
custom_commands/suggestions_preset.go flaky. To fix this, put the
refreshView call back to where it was (after the SetSelectedLineIdx
call), and instead insert an extra call to ReApplyFilter where necessary
to fix the bug that ff4ae4a544 was trying to fix.
I still don't think there was any user facing problem caused by this
(@ChrisMcD1 correct me if I'm wrong), so I don't think we need to cut a
0.46.1 release with the fix (otherwise it would have been a regression
in 0.46), and I only label it as `maintenance` because it only fixes CI.
Fixes#4267.
In ff4ae4a544 we changed the order of the calls to render before selecting the
branch. This was done only to save an extra call to ReApplyFilter, which is done
by refreshView; I claimed that the order of refreshView vs. SetSelectedLineIdx
doesn't matter here. I guess I was wrong about that, it makes the integration
test custom_commands/suggestions_preset.go flaky. To fix this, put the
refreshView call back to where it was (after the SetSelectedLineIdx call), and
instead insert an extra call to ReApplyFilter where necessary to fix the bug
that ff4ae4a544 was trying to fix.
- **PR Description**
This improves the user experience when users try to use an invalid key
name in their config, either for one of our standard keybindings or for
the key of a custom command.
Fixes half of #4256 (only the keybindings aspect of it, not the context
names).
