ronantakizawa/codereview-bench huggingface dataset:
Repository: zyedidia/micro
File: internal/action/actions.go (Go)

Code before:
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]
		lastC.OrigSelection[1] = lastC.CurSelection[1]
		lastC.Loc = lastC.CurSelection[1]

		h.Buf.MergeCursors()
		h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)
	} else {
		InfoBar.Message("No matches found")
	}
	h.Relocate()
	return true
}

// SkipMultiCursorBack moves the current multiple cursor to the previously available position
func (h *BufPane) SkipMultiCursorBack() bool {
	lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1)
	if !lastC.HasSelection() {
		return false
	}
	sel := lastC.GetSelection()
	searchStart := lastC.CurSelection[0]

	search := string(sel)
	search = regexp.QuoteMeta(search)
	if h.multiWord {
		search = "\\b" + search + "\\b"
	}

	match, found, err := h.Buf.FindNext(search, h.Buf.Start(), h.Buf.End(), searchStart, false, true)
	if err != nil {
		InfoBar.Error(err)
	}
	if found {
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]
		lastC.OrigSelection[1] = lastC.CurSelection[1]
		lastC.Loc = lastC.CurSelection[1]

		h.Buf.MergeCursors()
		h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)
	} else {
		InfoBar.Message("No matches found")
	}
	h.Relocate()
	return true
}

// RemoveMultiCursor removes the latest multiple cursor

Review comment (refactor):
It's almost exactly the same code as in `SkipMultiCursor()`, right? So it should be easy to move this code to a common helper function?

Code after:
	}
	if found {
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]
		lastC.OrigSelection[1] = lastC.CurSelection[1]
		lastC.Loc = lastC.CurSelection[1]

		h.Buf.MergeCursors()
		h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)
	} else {
		InfoBar.Message("No matches found")
	}
	h.Relocate()
	return true
}

// SkipMultiCursor moves the current multiple cursor to the next available position
func (h *BufPane) SkipMultiCursor() bool {
	return h.skipMultiCursor(true)
}

// SkipMultiCursorBack moves the current multiple cursor to the previous available position
func (h *BufPane) SkipMultiCursorBack() bool {
	return h.skipMultiCursor(false)
}

// RemoveMultiCursor removes the latest multiple cursor
func (h *BufPane) RemoveMultiCursor() bool {
	if h.Buf.NumCursors() > 1 {
		h.Buf.RemoveCursor(h.Buf.NumCursors() - 1)
		h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)
		h.Buf.UpdateCursors()
	} else if h.multiWord {
		h.multiWord = false
		h.Cursor.Deselect(true)
	} else {
		return false
	}
	h.Relocate()
	return true
}

// RemoveAllMultiCursors removes all cursors except the base cursor
func (h *BufPane) RemoveAllMultiCursors() bool {
	if h.Buf.NumCursors() > 1 || h.multiWord {
		h.Buf.ClearCursors()
		h.multiWord = false
	} else {
		return false
	}
Repository: zyedidia/micro
File: internal/action/actions.go (Go)

Code before:
	mx, my := e.Position()
	// ignore click on the status line
	if my >= h.BufView().Y+h.BufView().Height {
		return false
	}
	mouseLoc := h.LocFromVisual(buffer.Loc{X: mx, Y: my})

	if h.Buf.NumCursors() > 1 {
		cursors := h.Buf.GetCursors()
		for _, c := range cursors {
			if c.Loc == mouseLoc {
				h.Buf.RemoveCursor(c.Num)
				return true
			}
		}
	}

	c := buffer.NewCursor(b, mouseLoc)
	b.AddCursor(c)
	b.MergeCursors()

	return true
}

// SkipMultiCursor moves the current multiple cursor to the next or previous available position, depending on the argument `forward`.
func SkipMultiCursor(h *BufPane, forward bool) bool {
	lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1)
	if !lastC.HasSelection() {
		return false
	}
	sel := lastC.GetSelection()
	searchStart := lastC.CurSelection[1]
	if !forward {
		searchStart = lastC.CurSelection[0]
	}

	search := string(sel)
	search = regexp.QuoteMeta(search)
	if h.multiWord {
		search = "\\b" + search + "\\b"
	}

	match, found, err := h.Buf.FindNext(search, h.Buf.Start(), h.Buf.End(), searchStart, forward, true)
	if err != nil {
		InfoBar.Error(err)
	}
	if found {
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]
		lastC.OrigSelection[1] = lastC.CurSelection[1]

Review comment (refactor):
Erg, this looks ugly. I didn't meant this helper function not to be a method of `BufPane`.

The most obvious way is to name it `skipMultiCursor()` i.e. non-capitalized, i.e. private (it doesn't need to be public, right?). I.e.:

```
func (h *BufPane) skipMultiCursor(forward bool) bool
```

Code after:
	mx, my := e.Position()
	// ignore click on the status line
	if my >= h.BufView().Y+h.BufView().Height {
		return false
	}
	mouseLoc := h.LocFromVisual(buffer.Loc{X: mx, Y: my})

	if h.Buf.NumCursors() > 1 {
		cursors := h.Buf.GetCursors()
		for _, c := range cursors {
			if c.Loc == mouseLoc {
				h.Buf.RemoveCursor(c.Num)
				return true
			}
		}
	}

	c := buffer.NewCursor(b, mouseLoc)
	b.AddCursor(c)
	b.MergeCursors()

	return true
}

func (h *BufPane) skipMultiCursor(forward bool) bool {
	lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1)
	if !lastC.HasSelection() {
		return false
	}
	sel := lastC.GetSelection()
	searchStart := lastC.CurSelection[1]
	if !forward {
		searchStart = lastC.CurSelection[0]
	}

	search := string(sel)
	search = regexp.QuoteMeta(search)
	if h.multiWord {
		search = "\\b" + search + "\\b"
	}

	match, found, err := h.Buf.FindNext(search, h.Buf.Start(), h.Buf.End(), searchStart, forward, true)
	if err != nil {
		InfoBar.Error(err)
	}
	if found {
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]
		lastC.OrigSelection[1] = lastC.CurSelection[1]
		lastC.Loc = lastC.CurSelection[1]
Repository: zyedidia/micro
File: internal/action/actions.go (Go)

Code before:
	b := h.Buf
	mx, my := e.Position()
	// ignore click on the status line
	if my >= h.BufView().Y+h.BufView().Height {
		return false
	}
	mouseLoc := h.LocFromVisual(buffer.Loc{X: mx, Y: my})

	if h.Buf.NumCursors() > 1 {
		cursors := h.Buf.GetCursors()
		for _, c := range cursors {
			if c.Loc == mouseLoc {
				h.Buf.RemoveCursor(c.Num)
				return true
			}
		}
	}

	c := buffer.NewCursor(b, mouseLoc)
	b.AddCursor(c)
	b.MergeCursors()

	return true
}

// SkipMultiCursor moves the current multiple cursor to the next or previous available position, depending on the argument `forward`.
func (h *BufPane) skipMultiCursor(forward bool) bool {
	lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1)
	if !lastC.HasSelection() {
		return false
	}
	sel := lastC.GetSelection()
	searchStart := lastC.CurSelection[1]
	if !forward {
		searchStart = lastC.CurSelection[0]
	}

	search := string(sel)
	search = regexp.QuoteMeta(search)
	if h.multiWord {
		search = "\\b" + search + "\\b"
	}

	match, found, err := h.Buf.FindNext(search, h.Buf.Start(), h.Buf.End(), searchStart, forward, true)
	if err != nil {
		InfoBar.Error(err)
	}
	if found {
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]

Review comment (suggestion):
`SkipMultiCursor` still capitalized in this comment.

I think we don't need a comment here. It's an internal function, and from its usage below it is obvious what it does.

Code after:
	b := h.Buf
	mx, my := e.Position()
	// ignore click on the status line
	if my >= h.BufView().Y+h.BufView().Height {
		return false
	}
	mouseLoc := h.LocFromVisual(buffer.Loc{X: mx, Y: my})

	if h.Buf.NumCursors() > 1 {
		cursors := h.Buf.GetCursors()
		for _, c := range cursors {
			if c.Loc == mouseLoc {
				h.Buf.RemoveCursor(c.Num)
				return true
			}
		}
	}

	c := buffer.NewCursor(b, mouseLoc)
	b.AddCursor(c)
	b.MergeCursors()

	return true
}

func (h *BufPane) skipMultiCursor(forward bool) bool {
	lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1)
	if !lastC.HasSelection() {
		return false
	}
	sel := lastC.GetSelection()
	searchStart := lastC.CurSelection[1]
	if !forward {
		searchStart = lastC.CurSelection[0]
	}

	search := string(sel)
	search = regexp.QuoteMeta(search)
	if h.multiWord {
		search = "\\b" + search + "\\b"
	}

	match, found, err := h.Buf.FindNext(search, h.Buf.Start(), h.Buf.End(), searchStart, forward, true)
	if err != nil {
		InfoBar.Error(err)
	}
	if found {
		lastC.SetSelectionStart(match[0])
		lastC.SetSelectionEnd(match[1])
		lastC.OrigSelection[0] = lastC.CurSelection[0]
		lastC.OrigSelection[1] = lastC.CurSelection[1]
Repository: zyedidia/micro
File: pkg/highlight/parser.go (Go)

Code before:

		if _, ok := Groups[groupStr]; !ok {
			numGroups++
			Groups[groupStr] = numGroups
		}
		groupNum := Groups[groupStr]
		r.limitGroup = groupNum

		if err != nil {
			return nil, err
		}
	} else {
		r.limitGroup = r.group
	}

	// rules are optional
	if rules, ok := regionInfo["rules"]; ok {
		r.rules, err = parseRules(rules.([]interface{}), r)
		if err != nil {
			return nil, err
		}
	}

	if r.rules == nil {
		// allow empty rules
		r.rules = new(rules)
	}

	return r, nil
}


Review comment (suggestion):
nit: It seems a bit more idiomatic to use `&rules{}` instead of `new(rules)` in such cases. (Yeah it was me who first used `new(rules)` in #3213. We can change it to `&rules{}` there as well.)

Code after:

		if _, ok := Groups[groupStr]; !ok {
			numGroups++
			Groups[groupStr] = numGroups
		}
		groupNum := Groups[groupStr]
		r.limitGroup = groupNum

		if err != nil {
			return nil, err
		}
	} else {
		r.limitGroup = r.group
	}

	// rules are optional
	if rules, ok := regionInfo["rules"]; ok {
		r.rules, err = parseRules(rules.([]interface{}), r)
		if err != nil {
			return nil, err
		}
	}

	if r.rules == nil {
		// allow empty rules
		r.rules = &rules{}
	}

	return r, nil
}
Repository: zyedidia/micro
File: internal/util/profile.go (Go)

Code before:
package util

import (
	"fmt"
	"runtime"

	humanize "github.com/dustin/go-humanize"
)

// GetMemStats returns a string describing the memory usage and gc time used so far
func GetMemStats() string {
	var memstats runtime.MemStats
	runtime.ReadMemStats(&memstats)
	return fmt.Sprintf("Alloc: %s, Sys: %s, GC: %d, PauseTotalNs: %dns", humanize.Bytes(memstats.Alloc), humanize.Bytes(memstats.Sys), memstats.NumGC, memstats.PauseTotalNs)
}


Review comment (suggestion):
These may be actually occasionally useful for profiling?

BTW I didn't even know about the existence of this `internal/util/profile.go` until now (and I find it rather nice). (And BTW also I didn't know about the existence of the `memusage` command.)

Code after:

import (
	"fmt"
	"log"
	"runtime"
	"time"

	humanize "github.com/dustin/go-humanize"
)

// GetMemStats returns a string describing the memory usage and gc time used so far
func GetMemStats() string {
	var memstats runtime.MemStats
	runtime.ReadMemStats(&memstats)
	return fmt.Sprintf("Alloc: %s, Sys: %s, GC: %d, PauseTotalNs: %dns", humanize.Bytes(memstats.Alloc), humanize.Bytes(memstats.Sys), memstats.NumGC, memstats.PauseTotalNs)
}

func Tic(s string) time.Time {
	log.Println("START:", s)
	return time.Now()
}

func Toc(start time.Time) {
	end := time.Now()
	log.Println("END: ElapsedTime in seconds:", end.Sub(start))
}
Repository: zyedidia/micro
File: internal/action/command.go (Go)

Code before:
				if forceSplit {
					InfoBar.Error(errSplit)
					return
				}
				hsplit = true
				forceSplit = true
			default:
				topics = append(topics, arg)
			}
		}

		if len(topics) < 1 {
			// Do the same as without arg
			h.openHelp("help", hsplit, forceSplit)
			return
		}

		for _, topic := range topics {
			if config.FindRuntimeFile(config.RTHelp, topic) != nil {
				err := h.openHelp(topic, hsplit, forceSplit)
				if err != nil {
					InfoBar.Error(err)
				}
			} else {
				InfoBar.Error("Sorry, no help for ", topic)
			}
		}
	}
}

// VSplitCmd opens a vertical split with file given in the first argument
// If no file is given, it opens an empty buffer in a new split
func (h *BufPane) VSplitCmd(args []string) {
	if len(args) == 0 {
		// Open an empty vertical split
		h.VSplitAction()
		return
	}

	buf, err := buffer.NewBufferFromFile(args[0], buffer.BTDefault)
	if err != nil {
		InfoBar.Error(err)
		return
	}

	h.VSplitBuf(buf)
}

// HSplitCmd opens a horizontal split with file given in the first argument
// If no file is given, it opens an empty buffer in a new split
func (h *BufPane) HSplitCmd(args []string) {

Review comment (suggestion):
If current pane is a help pane and `forceSplit` is false, this opens just the last topic (in this current pane).

Something like this?

```
if len(topics) > 1 {
	forceSplit = true
}
```

Code after:
				hsplit = false
				forceSplit = true
			case "-hsplit":
				if forceSplit {
					InfoBar.Error(errSplit)
					return
				}
				hsplit = true
				forceSplit = true
			default:
				topics = append(topics, arg)
			}
		}

		if len(topics) < 1 {
			// Do the same as without arg
			h.openHelp("help", hsplit, forceSplit)
			return
		}
		if len(topics) > 1 {
			forceSplit = true
		}

		for _, topic := range topics {
			if config.FindRuntimeFile(config.RTHelp, topic) != nil {
				err := h.openHelp(topic, hsplit, forceSplit)
				if err != nil {
					InfoBar.Error(err)
				}
			} else {
				InfoBar.Error("Sorry, no help for ", topic)
			}
		}
	}
}

// VSplitCmd opens one or more vertical splits with the files given as arguments
// If no file is given, it opens an empty buffer in a new split
func (h *BufPane) VSplitCmd(args []string) {
	if len(args) == 0 {
		// Open an empty vertical split
		h.VSplitAction()
		return
	}

	for _, a := range args {
		buf, err := buffer.NewBufferFromFile(a, buffer.BTDefault)
		if err != nil {
			InfoBar.Error(err)
			return
		}
Repository: zyedidia/micro
File: internal/action/actions.go (Go)

Code before:

	if h.Cursor.HasSelection() {
		h.Cursor.Loc = h.Cursor.CurSelection[1]
		h.Cursor.ResetSelection()
		h.Cursor.StoreVisualX()
	}
	h.MoveCursorDown(h.BufView().Height)
	h.Relocate()
	return true
}

// HalfPageUp scrolls the view up half a page
func (h *BufPane) HalfPageUp() bool {
	h.ScrollUp(h.BufView().Height / 2)
	return true
}

// HalfPageDown scrolls the view down half a page
func (h *BufPane) HalfPageDown() bool {
	h.ScrollDown(h.BufView().Height / 2)
	h.ScrollAdjust()
	return true
}

// PageUpAndMoveCursor scrolls the view up a page and moves the cursor to keep it in view
func (h *BufPane) PageUpAndMoveCursor() bool {
	h.ScrollUp(h.BufView().Height)
	v := h.GetView()
	scrollmargin := int(h.Buf.Settings["scrollmargin"].(float64))
	h.Cursor.GotoLoc(buffer.Loc{0, v.StartLine.Line + scrollmargin})
	h.RemoveAllMultiCursors()
	h.Cursor.Deselect(true)
	return true
}

// PageDownAndMoveCursor scrolls the view down a page and moves the cursor to keep it in view
func (h *BufPane) PageDownAndMoveCursor() bool {
	h.ScrollDown(h.BufView().Height)
	h.ScrollAdjust()
	v := h.GetView()
	scrollmargin := int(h.Buf.Settings["scrollmargin"].(float64))
	h.Cursor.GotoLoc(buffer.Loc{0, v.StartLine.Line + scrollmargin})
	h.RemoveAllMultiCursors()
	h.Cursor.Deselect(true)
	return true
}

// ToggleDiffGutter turns the diff gutter off and on
func (h *BufPane) ToggleDiffGutter() bool {
	if !h.Buf.Settings["diffgutter"].(bool) {
		h.Buf.Settings["diffgutter"] = true

Review comment (suggestion):
While `MoveCursorToView` looks quite generic and useful, these 2 actions `PageUpAndMoveCursor` and `PageDownAndMoveCursor` don't look nice. It should be possible to use chained actions `PageUp,MoveCursorToView` and `PageDown,MoveCursorToView` instead, that's what chained actions are for.

Code after:
		h.Cursor.OrigSelection[0] = h.Cursor.Loc
	}
	h.MoveCursorUp(h.BufView().Height)
	h.Cursor.SelectTo(h.Cursor.Loc)
	h.Relocate()
	return true
}

// SelectPageDown selects down one page
func (h *BufPane) SelectPageDown() bool {
	if !h.Cursor.HasSelection() {
		h.Cursor.OrigSelection[0] = h.Cursor.Loc
	}
	h.MoveCursorDown(h.BufView().Height)
	h.Cursor.SelectTo(h.Cursor.Loc)
	h.Relocate()
	return true
}

// CursorPageUp places the cursor a page up
func (h *BufPane) CursorPageUp() bool {
	h.Cursor.Deselect(true)

	if h.Cursor.HasSelection() {
		h.Cursor.Loc = h.Cursor.CurSelection[0]
		h.Cursor.ResetSelection()
		h.Cursor.StoreVisualX()
	}
	h.MoveCursorUp(h.BufView().Height)
	h.Relocate()
	return true
}

// CursorPageDown places the cursor a page up
func (h *BufPane) CursorPageDown() bool {
	h.Cursor.Deselect(false)

	if h.Cursor.HasSelection() {
		h.Cursor.Loc = h.Cursor.CurSelection[1]
		h.Cursor.ResetSelection()
		h.Cursor.StoreVisualX()
	}
	h.MoveCursorDown(h.BufView().Height)
	h.Relocate()
	return true
}

// HalfPageUp scrolls the view up half a page
func (h *BufPane) HalfPageUp() bool {
	h.ScrollUp(h.BufView().Height / 2)
	return true
Repository: zyedidia/micro
File: internal/action/actions.go (Go)

Code before:
		h.Cursor.StoreVisualX()
	}
	h.MoveCursorDown(h.BufView().Height)
	h.Relocate()
	return true
}

// HalfPageUp scrolls the view up half a page
func (h *BufPane) HalfPageUp() bool {
	h.ScrollUp(h.BufView().Height / 2)
	return true
}

// HalfPageDown scrolls the view down half a page
func (h *BufPane) HalfPageDown() bool {
	h.ScrollDown(h.BufView().Height / 2)
	h.ScrollAdjust()
	return true
}

// PageUpAndMoveCursor scrolls the view up a page and moves the cursor to keep it in view
func (h *BufPane) PageUpAndMoveCursor() bool {
	h.ScrollUp(h.BufView().Height)
	v := h.GetView()
	scrollmargin := int(h.Buf.Settings["scrollmargin"].(float64))
	h.Cursor.GotoLoc(buffer.Loc{0, v.StartLine.Line + scrollmargin})
	h.RemoveAllMultiCursors()
	h.Cursor.Deselect(true)
	return true
}

// PageDownAndMoveCursor scrolls the view down a page and moves the cursor to keep it in view
func (h *BufPane) PageDownAndMoveCursor() bool {
	h.ScrollDown(h.BufView().Height)
	h.ScrollAdjust()
	v := h.GetView()
	scrollmargin := int(h.Buf.Settings["scrollmargin"].(float64))
	h.Cursor.GotoLoc(buffer.Loc{0, v.StartLine.Line + scrollmargin})
	h.RemoveAllMultiCursors()
	h.Cursor.Deselect(true)
	return true
}

// ToggleDiffGutter turns the diff gutter off and on
func (h *BufPane) ToggleDiffGutter() bool {
	if !h.Buf.Settings["diffgutter"].(bool) {
		h.Buf.Settings["diffgutter"] = true
		h.Buf.UpdateDiff()
		InfoBar.Message("Enabled diff gutter")
	} else {
		h.Buf.Settings["diffgutter"] = false

Review comment (suggestion):
This doesn't take soft-wrapped lines into account. The correct way is:

```
	h.Cursor.GotoLoc(h.LocFromVLoc(display.VLoc{
		SLoc:    h.Scroll(v.StartLine, scrollmargin),
		VisualX: 0,
	}))
```

Code after:
	h.Relocate()
	return true
}

// SelectPageDown selects down one page
func (h *BufPane) SelectPageDown() bool {
	if !h.Cursor.HasSelection() {
		h.Cursor.OrigSelection[0] = h.Cursor.Loc
	}
	h.MoveCursorDown(h.BufView().Height)
	h.Cursor.SelectTo(h.Cursor.Loc)
	h.Relocate()
	return true
}

// CursorPageUp places the cursor a page up
func (h *BufPane) CursorPageUp() bool {
	h.Cursor.Deselect(true)

	if h.Cursor.HasSelection() {
		h.Cursor.Loc = h.Cursor.CurSelection[0]
		h.Cursor.ResetSelection()
		h.Cursor.StoreVisualX()
	}
	h.MoveCursorUp(h.BufView().Height)
	h.Relocate()
	return true
}

// CursorPageDown places the cursor a page up
func (h *BufPane) CursorPageDown() bool {
	h.Cursor.Deselect(false)

	if h.Cursor.HasSelection() {
		h.Cursor.Loc = h.Cursor.CurSelection[1]
		h.Cursor.ResetSelection()
		h.Cursor.StoreVisualX()
	}
	h.MoveCursorDown(h.BufView().Height)
	h.Relocate()
	return true
}

// HalfPageUp scrolls the view up half a page
func (h *BufPane) HalfPageUp() bool {
	h.ScrollUp(h.BufView().Height / 2)
	return true
}

// HalfPageDown scrolls the view down half a page
func (h *BufPane) HalfPageDown() bool {
Repository: zyedidia/micro
File: internal/action/actions.go (Go)

Code before:
		VisualX: 0,
	}))
	return true
}

// CursorToViewCenter moves the cursor to the center of the view
func (h *BufPane) CursorToViewCenter() bool {
	v := h.GetView()
	h.Buf.ClearCursors()
	h.Cursor.GotoLoc(h.LocFromVLoc(display.VLoc{
		SLoc:    h.Scroll(v.StartLine, h.BufView().Height/2),
		VisualX: 0,
	}))
	return true
}

// CursorToViewBottom moves the cursor to the bottom of the view,
// offset by scrollmargin unless at the beginning or end of the file
func (h *BufPane) CursorToViewBottom() bool {
	v := h.GetView()
	h.Buf.ClearCursors()
	scrollmargin := int(h.Buf.Settings["scrollmargin"].(float64))
	height := h.BufView().Height
	bEnd := h.SLocFromLoc(h.Buf.End())
	lastLine := h.Scroll(v.StartLine, height-1-scrollmargin)
	if bEnd.Line-lastLine.Line < scrollmargin {
		scrollmargin = 0
	}
	h.Cursor.GotoLoc(h.LocFromVLoc(display.VLoc{
		SLoc:    h.Scroll(v.StartLine, height-1-scrollmargin),
		VisualX: 0,
	}))
	return true
}

// MoveCursorUp is not an action
func (h *BufPane) MoveCursorUp(n int) {
	if !h.Buf.Settings["softwrap"].(bool) {
		h.Cursor.UpN(n)
	} else {
		vloc := h.VLocFromLoc(h.Cursor.Loc)
		sloc := h.Scroll(vloc.SLoc, -n)
		if sloc == vloc.SLoc {
			// we are at the beginning of buffer
			h.Cursor.Loc = h.Buf.Start()
			h.Cursor.LastVisualX = 0
		} else {
			vloc.SLoc = sloc
			vloc.VisualX = h.Cursor.LastVisualX
			h.Cursor.Loc = h.LocFromVLoc(vloc)
		}

Review comment (suggestion):
And same problem here...

Once again, `bEnd.Line-lastLine.Line` may be less than `scrollmargin` yet span 10000 visual lines.

Code after:
		SLoc:    h.Scroll(v.StartLine, scrollmargin),
		VisualX: 0,
	}))
	return true
}

// CursorToViewCenter moves the cursor to the center of the view
func (h *BufPane) CursorToViewCenter() bool {
	v := h.GetView()
	h.Buf.ClearCursors()
	h.Cursor.GotoLoc(h.LocFromVLoc(display.VLoc{
		SLoc:    h.Scroll(v.StartLine, h.BufView().Height/2),
		VisualX: 0,
	}))
	return true
}

// CursorToViewBottom moves the cursor to the bottom of the view,
// offset by scrollmargin unless at the beginning or end of the file
func (h *BufPane) CursorToViewBottom() bool {
	v := h.GetView()
	h.Buf.ClearCursors()
	scrollmargin := int(h.Buf.Settings["scrollmargin"].(float64))
	bEnd := h.SLocFromLoc(h.Buf.End())
	lastLine := h.Scroll(v.StartLine, h.BufView().Height-1)
	if lastLine == bEnd {
		scrollmargin = 0
	}
	h.Cursor.GotoLoc(h.LocFromVLoc(display.VLoc{
		SLoc:    h.Scroll(lastLine, -scrollmargin),
		VisualX: 0,
	}))
	return true
}

// MoveCursorUp is not an action
func (h *BufPane) MoveCursorUp(n int) {
	if !h.Buf.Settings["softwrap"].(bool) {
		h.Cursor.UpN(n)
	} else {
		vloc := h.VLocFromLoc(h.Cursor.Loc)
		sloc := h.Scroll(vloc.SLoc, -n)
		if sloc == vloc.SLoc {
			// we are at the beginning of buffer
			h.Cursor.Loc = h.Buf.Start()
			h.Cursor.LastVisualX = 0
		} else {
			vloc.SLoc = sloc
			vloc.VisualX = h.Cursor.LastVisualX
			h.Cursor.Loc = h.LocFromVLoc(vloc)
		}