better handling of cursor and origin positionings

3 files changed, 52 insertions, 22 deletions

diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go
index bfcc938bb..8fd2fb30c 100644
--- a/pkg/gui/files_panel.go
+++ b/pkg/gui/files_panel.go
@@ -275,22 +275,23 @@ func (gui *Gui) updateHasMergeConflictStatus() error {
 	return nil
 }
 
-func (gui *Gui) renderFile(file commands.File, filesView *gocui.View) {
+func (gui *Gui) renderFile(file commands.File) string {
 	// potentially inefficient to be instantiating these color
 	// objects with each render
 	red := color.New(color.FgRed)
 	green := color.New(color.FgGreen)
 	if !file.Tracked && !file.HasStagedChanges {
-		red.Fprintln(filesView, file.DisplayString)
-		return
+		return red.Sprint(file.DisplayString)
 	}
-	green.Fprint(filesView, file.DisplayString[0:1])
-	red.Fprint(filesView, file.DisplayString[1:3])
+
+	output := green.Sprint(file.DisplayString[0:1])
+	output += red.Sprint(file.DisplayString[1:3])
 	if file.HasUnstagedChanges {
-		red.Fprintln(filesView, file.Name)
+		output += red.Sprint(file.Name)
 	} else {
-		green.Fprintln(filesView, file.Name)
+		output += green.Sprint(file.Name)
 	}
+	return output
 }
 
 func (gui *Gui) catSelectedFile(g *gocui.Gui) (string, error) {
@@ -319,8 +320,14 @@ func (gui *Gui) refreshFiles(g *gocui.Gui) error {
 	}
 	gui.refreshStateFiles()
 	filesView.Clear()
-	for _, file := range gui.State.Files {
-		gui.renderFile(file, filesView)
+	for i, file := range gui.State.Files {
+		str := gui.renderFile(file)
+		if i < len(gui.State.Files)-1 {
+			str += "\n"
+		}
+		if _, err := filesView.Write([]byte(str)); err != nil {
+			return err
+		}
 	}
 	gui.correctCursor(filesView)
 	if filesView == g.CurrentView() {
diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go
index 68041390d..f52ceda07 100644
--- a/pkg/gui/menu_panel.go
+++ b/pkg/gui/menu_panel.go
@@ -8,6 +8,12 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
+// I need to store the handler function in state and it will take an interface and do something with it
+// I need to have another function describing how to display one of the structs
+// perhaps this calls for an interface where the struct is Binding and the interface has the methods Display and Execute
+// but this means that for the one struct I can only have one possible display/execute function, but I want to use whatever I want.
+// Would I ever need to use different handlers for different things? Maybe I should assume not given that I can cross that bridge when I come to it
+
 func (gui *Gui) handleMenuPress(g *gocui.Gui, v *gocui.View) error {
 	lineNumber := gui.getItemPosition(v)
 	if gui.State.Keys[lineNumber].Key == nil {
@@ -106,11 +112,11 @@ func (gui *Gui) handleMenu(g *gocui.Gui, v *gocui.View) error {
 	content := append(contentPanel, contentGlobal...)
 	gui.State.Keys = append(bindingsPanel, bindingsGlobal...)
 	// append newline at the end so the last line would be selectable
-	contentJoined := strings.Join(content, "\n") + "\n"
+	contentJoined := strings.Join(content, "\n")
 
 	// y1-1 so there will not be an extra space at the end of panel
 	x0, y0, x1, y1 := gui.getConfirmationPanelDimensions(g, contentJoined)
-	menuView, _ := g.SetView("menu", x0, y0, x1, y1-1, 0)
+	menuView, _ := g.SetView("menu", x0, y0, x1, y1, 0)
 	menuView.Title = strings.Title(gui.Tr.SLocalize("menu"))
 	menuView.FgColor = gocui.ColorWhite
 
diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go
index 7a5bd8874..5178bd4d9 100644
--- a/pkg/gui/view_helpers.go
+++ b/pkg/gui/view_helpers.go
@@ -160,7 +160,6 @@ func (gui *Gui) getItemPosition(v *gocui.View) int {
 
 func (gui *Gui) cursorUp(g *gocui.Gui, v *gocui.View) error {
 	// swallowing cursor movements in main
-	// TODO: pull this out
 	if v == nil || v.Name() == "main" {
 		return nil
 	}
@@ -179,19 +178,28 @@ func (gui *Gui) cursorUp(g *gocui.Gui, v *gocui.View) error {
 
 func (gui *Gui) cursorDown(g *gocui.Gui, v *gocui.View) error {
 	// swallowing cursor movements in main
-	// TODO: pull this out
 	if v == nil || v.Name() == "main" {
 		return nil
 	}
 	cx, cy := v.Cursor()
 	ox, oy := v.Origin()
-	if cy+oy >= len(v.BufferLines())-2 {
+	ly := len(v.BufferLines()) - 1
+	_, height := v.Size()
+	maxY := height - 1
+
+	// if we are at the end we just return
+	if cy+oy == ly {
 		return nil
 	}
-	if err := v.SetCursor(cx, cy+1); err != nil {
-		if err := v.SetOrigin(ox, oy+1); err != nil {
-			return err
-		}
+
+	var err error
+	if cy < maxY {
+		err = v.SetCursor(cx, cy+1)
+	} else {
+		err = v.SetOrigin(ox, oy+1)
+	}
+	if err != nil {
+		return err
 	}
 
 	gui.newLineFocused(g, v)
@@ -208,10 +216,19 @@ func (gui *Gui) resetOrigin(v *gocui.View) error {
 // if the cursor down past the last item, move it to the last line
 func (gui *Gui) correctCursor(v *gocui.View) error {
 	cx, cy := v.Cursor()
-	_, oy := v.Origin()
-	lineCount := len(v.BufferLines()) - 2
-	if cy >= lineCount-oy {
-		return v.SetCursor(cx, lineCount-oy)
+	ox, oy := v.Origin()
+	_, height := v.Size()
+	maxY := height - 1
+	ly := len(v.BufferLines()) - 1
+	if oy+cy <= ly {
+		return nil
+	}
+	newCy := utils.Min(ly, maxY)
+	if err := v.SetCursor(cx, newCy); err != nil {
+		return err
+	}
+	if err := v.SetOrigin(ox, ly-newCy); err != nil {
+		return err
 	}
 	return nil
 }