From f2963ec0967878d3b63f47ac457b17e4660b718c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 1 Dec 2024 10:32:18 +0100 Subject: [PATCH] Make view geometry a little more sane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- gui.go | 2 +- view.go | 61 +++++++++++++++++++++++++++------------------------------ 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/gui.go b/gui.go index 644447d..0ea2a73 100644 --- a/gui.go +++ b/gui.go @@ -912,7 +912,7 @@ func calcScrollbarRune( } func calcRealScrollbarStartEnd(v *View) (bool, int, int) { - height := v.InnerHeight() + 1 + height := v.InnerHeight() fullHeight := v.ViewLinesHeight() - v.scrollMargin() if v.CanScrollPastBottom { diff --git a/view.go b/view.go index c3e183a..190a653 100644 --- a/view.go +++ b/view.go @@ -322,14 +322,9 @@ func (v *View) FocusPoint(cx int, cy int) { if cy < 0 || cy > lineCount { return } - _, height := v.Size() + height := v.InnerHeight() - ly := height - 1 - if ly < 0 { - ly = 0 - } - - v.oy = calculateNewOrigin(cy, v.oy, lineCount, ly) + v.oy = calculateNewOrigin(cy, v.oy, lineCount, height) v.cx = cx v.cy = cy - v.oy } @@ -343,16 +338,16 @@ func (v *View) CancelRangeSelect() { } func calculateNewOrigin(selectedLine int, oldOrigin int, lineCount int, viewHeight int) int { - if viewHeight > lineCount { + if viewHeight >= lineCount { return 0 - } else if selectedLine < oldOrigin || selectedLine > oldOrigin+viewHeight { + } else if selectedLine < oldOrigin || selectedLine >= oldOrigin+viewHeight { // If the selected line is outside the visible area, scroll the view so // that the selected line is in the middle. newOrigin := selectedLine - viewHeight/2 // However, take care not to overflow if the total line count is less // than the view height. - maxOrigin := lineCount - viewHeight - 1 + maxOrigin := lineCount - viewHeight if newOrigin > maxOrigin { newOrigin = maxOrigin } @@ -438,22 +433,32 @@ func (v *View) Dimensions() (int, int, int, int) { return v.x0, v.y0, v.x1, v.y1 } -// Size returns the number of visible columns and rows in the View. +// Size returns the number of visible columns and rows in the View, including +// the frame if any func (v *View) Size() (x, y int) { return v.Width(), v.Height() } +// InnerSize returns the number of usable columns and rows in the View, excluding +// the frame if any +func (v *View) InnerSize() (x, y int) { + return v.InnerWidth(), v.InnerHeight() +} + func (v *View) Width() int { - return v.x1 - v.x0 - 1 + return v.x1 - v.x0 + 1 } func (v *View) Height() int { - return v.y1 - v.y0 - 1 + return v.y1 - v.y0 + 1 } -// if a view has a frame, that leaves less space for its writeable area +// The writeable area of the view is always two less then the view's size, +// because if it has a frame, we need to subtract that, but if it doesn't, the +// view is made 1 larger on all sides. I'd like to clean this up at some point, +// but for now we live with this weirdness. func (v *View) InnerWidth() int { - innerWidth := v.Width() - v.frameOffset() + innerWidth := v.Width() - 2 if innerWidth < 0 { return 0 } @@ -462,7 +467,7 @@ func (v *View) InnerWidth() int { } func (v *View) InnerHeight() int { - innerHeight := v.Height() - v.frameOffset() + innerHeight := v.Height() - 2 if innerHeight < 0 { return 0 } @@ -470,14 +475,6 @@ func (v *View) InnerHeight() int { return innerHeight } -func (v *View) frameOffset() int { - if v.Frame { - return 1 - } else { - return 0 - } -} - // Name returns the name of the view. func (v *View) Name() string { return v.name @@ -573,7 +570,7 @@ func max(a, b int) int { // SetCursor sets the cursor position of the view at the given point, // relative to the view. It checks if the position is valid. func (v *View) SetCursor(x, y int) { - maxX, maxY := v.Size() + maxX, maxY := v.InnerSize() if x < 0 || x >= maxX || y < 0 || y >= maxY { return } @@ -582,7 +579,7 @@ func (v *View) SetCursor(x, y int) { } func (v *View) SetCursorX(x int) { - maxX, _ := v.Size() + maxX := v.InnerWidth() if x < 0 || x >= maxX { return } @@ -590,7 +587,7 @@ func (v *View) SetCursorX(x int) { } func (v *View) SetCursorY(y int) { - _, maxY := v.Size() + maxY := v.InnerHeight() if y < 0 || y >= maxY { return } @@ -917,7 +914,7 @@ func (v *View) parseInput(ch rune, x int, _ int) (bool, []cell) { for _, cell := range v.lines[v.wy][0:v.wx] { cx += runewidth.RuneWidth(cell.chr) } - repeatCount = v.InnerWidth() - cx + 1 + repeatCount = v.InnerWidth() - cx ch = ' ' truncateLine = true } else if isEscape { @@ -1157,7 +1154,7 @@ func (v *View) draw() { v.clearRunes() - maxX, maxY := v.Size() + maxX, maxY := v.InnerSize() if v.Wrap { if maxX == 0 { @@ -1250,7 +1247,7 @@ func (v *View) draw() { func (v *View) refreshViewLinesIfNeeded() { if v.tainted { - maxX := v.Width() + maxX := v.InnerWidth() lineIdx := 0 lines := v.lines if v.HasLoader { @@ -1341,7 +1338,7 @@ func (v *View) realPosition(vx, vy int) (x, y int, ok bool) { // clearRunes erases all the cells in the view. func (v *View) clearRunes() { - maxX, maxY := v.Size() + maxX, maxY := v.InnerSize() for x := 0; x < maxX; x++ { for y := 0; y < maxY; y++ { tcellSetCell(v.x0+x+1, v.y0+y+1, ' ', v.FgColor, v.BgColor, v.outMode) @@ -1789,7 +1786,7 @@ func (v *View) adjustDownwardScrollAmount(scrollHeight int) int { _, oy := v.Origin() y := oy if !v.CanScrollPastBottom { - _, sy := v.Size() + sy := v.InnerHeight() y += sy } scrollableLines := v.ViewLinesHeight() - y