fix: don't reserve space for trailing divider

[CatsDeservePets]

• Refs [Feature Request] Border Spacing option #2190

[CatsDeservePets]

This might not be the right way after all. Instead, the getWidths() function should be updated.

[joelim-work]

This might not be the right way after all. Instead, the getWidths() function should be updated.

I agree, the divider should not be part of the width of each window. So wtot should be reduced by the number of windows - 1 before calculating the ratios. If drawbox is enabled, then another two spaces should be taken out as they will be used for the left and right borders.

Also getWidths could be unit tested (maybe move to misc.go?), and the global options could be passed in as parameters.

[joelim-work]

Hi @CatsDeservePets, I'm not sure if you spent any more time on this, but I ended up with the patch below. Also I don't insist on unit tests, it was just a suggestion.

diff --git a/ui.go b/ui.go
index f7bcca9..b9a7efc 100644
--- a/ui.go
+++ b/ui.go
@@ -480,10 +480,6 @@ func (win *win) printDir(ui *ui, dir *dir, context *dirContext, dirStyle *dirSty
 
 		// make space for select marker, and leave another space at the end
 		maxWidth := win.w - lnwidth - 2
-		// make extra space to separate windows if drawbox is not enabled
-		if !gOpts.drawbox {
-			maxWidth--
-		}
 
 		var icon []rune
 		var iconDef iconDef
@@ -599,21 +595,23 @@ func getCustomWidth(dir *dir, beg, end int) int {
 }
 
 func getWidths(wtot int) []int {
+	wlen := len(gOpts.ratios)
+	wtot -= (wlen - 1)
+	if gOpts.drawbox {
+		wtot -= 2
+	}
+	wtot = max(wtot, 0)
+
 	rsum := 0
 	for _, r := range gOpts.ratios {
 		rsum += r
 	}
 
-	wlen := len(gOpts.ratios)
 	widths := make([]int, wlen)
 
-	if gOpts.drawbox {
-		wtot -= (wlen + 1)
-	}
-
 	wsum := 0
 	for i := range wlen - 1 {
-		widths[i] = gOpts.ratios[i] * wtot / rsum
+		widths[i] = wtot * gOpts.ratios[i] / rsum
 		wsum += widths[i]
 	}
 	widths[wlen-1] = wtot - wsum
@@ -624,19 +622,20 @@ func getWidths(wtot int) []int {
 func getWins(screen tcell.Screen) []*win {
 	wtot, htot := screen.Size()
 
-	widths := getWidths(wtot)
+	h := max(htot-2, 0)
+	x := 0
+	y := 1
+	if gOpts.drawbox {
+		h = max(htot-4, 0)
+		x = 1
+		y = 2
+	}
 
-	wacc := 0
-	wlen := len(widths)
-	wins := make([]*win, 0, wlen)
-	for i := range wlen {
-		if gOpts.drawbox {
-			wacc++
-			wins = append(wins, newWin(widths[i], max(htot-4, 0), wacc, 2))
-		} else {
-			wins = append(wins, newWin(widths[i], max(htot-2, 0), wacc, 1))
-		}
-		wacc += widths[i]
+	widths := getWidths(wtot)
+	wins := make([]*win, 0, len(widths))
+	for _, w := range widths {
+		wins = append(wins, newWin(w, h, x, y))
+		x += w + 1
 	}
 
 	return wins

[CatsDeservePets]

Hi @joelim-work!
I was kinda busy this week so I did not do much. I moved getWidths to misc.go and used parameters instead of globals. My actual semantic changes (not committed yet) are these:

diff --git a/misc.go b/misc.go
index 68af7cb..90b6e24 100644
--- a/misc.go
+++ b/misc.go
@@ -522,8 +522,9 @@ func getWidths(wtot int, ratios []int, drawBox bool) []int {
 	wlen := len(ratios)
 	widths := make([]int, wlen)
 
+	wtot -= wlen - 1
 	if drawBox {
-		wtot -= (wlen + 1)
+		wtot -= 2
 	}
 
 	wsum := 0
diff --git a/ui.go b/ui.go
index 59c1db7..8df431c 100644
--- a/ui.go
+++ b/ui.go
@@ -480,10 +480,6 @@ func (win *win) printDir(ui *ui, dir *dir, context *dirContext, dirStyle *dirSty
 
 		// make space for select marker, and leave another space at the end
 		maxWidth := win.w - lnwidth - 2
-		// make extra space to separate windows if drawbox is not enabled
-		if !gOpts.drawbox {
-			maxWidth--
-		}
 
 		var icon []rune
 		var iconDef iconDef
@@ -614,6 +610,9 @@ func getWins(screen tcell.Screen) []*win {
 			wins = append(wins, newWin(widths[i], max(htot-2, 0), wacc, 1))
 		}
 		wacc += widths[i]
+		if !gOpts.drawbox && i < wlen-1 {
+			wacc++
+		}
 	}
 
 	return wins

Even without you suggesting it, I thought to myself that this should probably have some tests added to it. One reason for this is that I also thought about changing the way widths get calculated in general. I don't like how the last window gets the remaining space as seen here (completely subjective of course):

ratios: []int{1, 2, 2}, widths: []int{16, 33, 35}
ratios: []int{1, 2, 2}, widths: []int{18, 36, 38}
ratios: []int{1, 2, 2}, widths: []int{19, 39, 40}
ratios: []int{1, 2, 2}, widths: []int{20, 41, 43}
ratios: []int{1, 2, 2}, widths: []int{22, 44, 46}
ratios: []int{1, 2, 2}, widths: []int{25, 50, 51}
ratios: []int{1, 2, 2}, widths: []int{27, 54, 56}
ratios: []int{1, 2, 2}, widths: []int{30, 60, 60}

The current behaviour causes constant re-sizing when zooming inside the terminal window.

[joelim-work]

Both of the diffs are effectively doing the same thing, it's just that my one rewrites the surrounding code while your one minimizes the amount of changed lines. I generally prefer to clean up (subjective opinion) the code as I go along.

I also thought about changing the way widths get calculated in general. I don't like how the last window gets the remaining space as seen here (completely subjective of course):

ratios: []int{1, 2, 2}, widths: []int{16, 33, 35}
ratios: []int{1, 2, 2}, widths: []int{18, 36, 38}
ratios: []int{1, 2, 2}, widths: []int{19, 39, 40}
ratios: []int{1, 2, 2}, widths: []int{20, 41, 43}
ratios: []int{1, 2, 2}, widths: []int{22, 44, 46}
ratios: []int{1, 2, 2}, widths: []int{25, 50, 51}
ratios: []int{1, 2, 2}, widths: []int{27, 54, 56}
ratios: []int{1, 2, 2}, widths: []int{30, 60, 60}

I'm not 100% sure if I understood your comment here correctly. It is true that there is special logic for the last window to get the remaining space, but that is because of rounding issues. Consider applying ratios 1:1:1 to a width of 80 cells - in this case two cells will be removed for the dividers and each windows will end up as {26, 26, 26}, which is perfect. Now imagine the width is changed to 81 cells - after removing the dividers there will be 79 cells left over which cannot be evenly distributed among three windows, so either the entire space cannot be used, or the ratios will not be applied exactly.

One possible idea you can consider is to use rounding instead of flooring (via integer division) to get accurate widths.

EDIT: Another thing is that when calculating the width of each window separately, rounding errors can add up, so maybe it's better to use the 'remaining space' technique for each window instead of only the last one, something like below:

func getWidths(wtot int) []int {
	rtot := 0
	for _, r := range gOpts.ratios {
		rtot += r
	}

	wlen := len(gOpts.ratios)
	widths := make([]int, wlen)

	wtot -= wlen - 1
	if gOpts.drawbox {
		wtot -= -2
	}

	// for each window, calculate where the window is supposed to end, then
	// subtract the total width of all previous windows to get the width
	rsum := 0
	wsum := 0
	for i, r := range gOpts.ratios {
		rsum += r
		widths[i] = wtot*rsum/rtot - wsum
		wsum += widths[i]
	}

	return widths
}