Change side panel width calculation to work for larger numbers #4287
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrisMcD1
commented
Feb 19, 2025
| } else { | ||
| mainSectionWeight = 1 | ||
| mainSectionWeight = sideSectionWeight |
There was a problem hiding this comment.
These should all be equivalent (and are if the tests prove anything), since the mainSectionWeight was always compared to the static side section 1 before. It will maintain its 1:1 and 1:2 ratios with this code.
| // we could make this better by creating ratios like 2:3 rather than always 1:something | ||
| mainSectionWeight := int(1/sidePanelWidthRatio) - 1 | ||
| sideSectionWeight := 1 | ||
| // Using 24 so that the default of 0.3333 will remain consistent with previous behavior |
There was a problem hiding this comment.
Is the idea that we want this number to have many factors? If so, how about we add 5 as a factor to it to make 120?
Also we should have this value assigned to a variable rather than duplicate it
There was a problem hiding this comment.
Yeah, the idea is to make it have many factors.
Moved the value to a constant, and made it 120. I am still a bit confused what happened when I first wrote this where having the const multiplier be too high broke the gui, but that doesn't seem to be the case for 120!
ChrisMcD1
force-pushed
the
3721-sidePanelWidth
branch
from
2854187 to
f73b618
Compare
|
Ah, shifting to 120 changed the 0.8 test I wrote marginally. Fixed! |
ChrisMcD1
force-pushed
the
3721-sidePanelWidth
branch
2 times, most recently
from
32e18cd to
2f03526
Compare
|
@jesseduffield Fixed up the broken tests (again). I didn't notice the pipelines were failing because the failing check-required-label check makes all of my PRs have a red X on them from the overview screen. Is there some way I can add a |
|
@ChrisMcD1 for now, Stefan and I can handle the labels as one of us is going to be reviewing your PRs |
This technically is a breaking change for some existing numbers, but it stays the same for default case, and isn't much different for others
jesseduffield
force-pushed
the
3721-sidePanelWidth
branch
from
2f03526 to
9d07404
Compare
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Feb 26, 2025
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | patch | `v0.47.1` -> `v0.47.2` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary> ### [`v0.47.2`](https://github.com/jesseduffield/lazygit/releases/tag/v0.47.2) [Compare Source](jesseduffield/lazygit@v0.47.1...v0.47.2) <!-- Release notes generated using configuration in .github/release.yml at v0.47.2 --> Small patch release for you all. This is mainly to fix an issue with v0.47.1 which erroneously re-indented users' lazygit config files on startup. Shout-out to [@​karimkhaleel](https://github.com/karimkhaleel) for his MR with some gnarly yaml-handling code. And a special shout-out to [@​ChrisMcD1](https://github.com/ChrisMcD1) who has been pumping out many great contributions lately. Great to have you aboard. #### What's Changed ##### Enhancements 🔥 - Skip post-checkout hook when discarding changes by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4313 ##### Fixes 🔧 - fix: Disable global keybinds when confirmation is active by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4284 - Don't rewrite config file unnecessarily when it contains commitPrefixes by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4311 - Change side panel width calculation to work for larger numbers by [@​ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4287 ##### Maintenance ⚙️ - Fix auto-release schedule by [@​jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4308 - Use indentation of 2 when rewriting auto-migrated config file by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4312 - Use refs in jsonschema by [@​karimkhaleel](https://github.com/karimkhaleel) in jesseduffield/lazygit#4309 - Improve release workflow by [@​jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4307 - Filter out dev comments from schema by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4319 - Fix release script by [@​jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4322 - Fix release script once again by [@​jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4323 ##### Docs 📖 - Improve the error message when users have gpg signing turned on by [@​stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4296 **Full Changelog**: jesseduffield/lazygit@v0.47.1...v0.47.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODAuMiIsInVwZGF0ZWRJblZlciI6IjM5LjE4MC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation of calculating sidePanelWidths does not support any number higher than 0.5. Past that point,
mainSectionWidthwill always be 0 because1/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:
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.
Fixes #3721
go generate ./...)