Skip to content

Conversation

@stefanhaller
Copy link
Collaborator

This is a breaking change!

This fixes View.Size, Width and Height to be the correct (outer) size of a view including its frame, and InnerSize/InnerWidth/InnerHeight to be the usable client area exluding the frame. See commit message for details.

Reviewed as part of jesseduffield/lazygit#4085.

This is a breaking change!

This fixes View.Size, Width and Height to be the correct (outer) size of a view
including its frame, and InnerSize/InnerWidth/InnerHeight to be the usable
client area exluding the frame. Previously, Size was actually the InnerSize (and
a lot of client code used it as such, so these need to be changed to InnerSize).
InnerSize, on the other hand, was *one* less than Size (not two, as you would
have expected), and in many cases this was made up for at call sites by adding 1
(e.g. in calcRealScrollbarStartEnd, parseInput, and many other places in the
lazygit code).

There are still some weird things left that I didn't address here:
- a view's lower-right coordinates (x1/y1) are one less than you would expect.
  For example, a view with a 2x2 client area like this:
    ╭──╮
    │ab│
    │cd│
    ╰──╯
  in the top-left corner of the screen (x0 and y0 both zero) has x1/xy at 3, not
  4 as would be more natural.
- a view without a frame has its coordinates extended by 1 on all sides; to
  illustrate, the same 2x2 view as before but without a frame, sitting in the
  top-left corder of the screen, has coordinates x0=-1, y0=-1, x1=2, y1=2. This
  is highly confusing and unexpected.

I left these as they are because they would be even more of a breaking change,
and also because they don't have quite as much of an impact on general app code.
@stefanhaller stefanhaller merged commit 68c437b into master Dec 1, 2024
1 check passed
@stefanhaller stefanhaller deleted the saner-view-geometry branch December 1, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants