From 85afb6eb878df72abf55806f81ee90def9b39c1b Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 13 Oct 2024 17:46:34 +0200 Subject: [PATCH 1/4] Use StoreVisualX() all over the code Since we already have the StoreVisualX() helper, use it all over the place instead of setting LastVisualX directly. This will allow us to add more logic to StoreVisualX() add let this extra logic apply everywhere automatically. --- internal/action/actions.go | 4 ++-- internal/buffer/cursor.go | 4 ++-- internal/buffer/eventhandler.go | 2 +- runtime/plugins/comment/comment.lua | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index cf6d954f..ccfb686b 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -657,7 +657,7 @@ func (h *BufPane) InsertNewline() bool { h.Buf.Remove(buffer.Loc{X: 0, Y: h.Cursor.Y - 1}, buffer.Loc{X: util.CharacterCount(line), Y: h.Cursor.Y - 1}) } } - h.Cursor.LastVisualX = h.Cursor.GetVisualX() + h.Cursor.StoreVisualX() h.Relocate() return true } @@ -687,7 +687,7 @@ func (h *BufPane) Backspace() bool { h.Buf.Remove(loc.Move(-1, h.Buf), loc) } } - h.Cursor.LastVisualX = h.Cursor.GetVisualX() + h.Cursor.StoreVisualX() h.Relocate() return true } diff --git a/internal/buffer/cursor.go b/internal/buffer/cursor.go index 7070dc23..784f2d39 100644 --- a/internal/buffer/cursor.go +++ b/internal/buffer/cursor.go @@ -100,7 +100,7 @@ func (c *Cursor) GetCharPosInLine(b []byte, visualPos int) int { // Start moves the cursor to the start of the line it is on func (c *Cursor) Start() { c.X = 0 - c.LastVisualX = c.GetVisualX() + c.StoreVisualX() } // StartOfText moves the cursor to the first non-whitespace rune of @@ -131,7 +131,7 @@ func (c *Cursor) IsStartOfText() bool { // End moves the cursor to the end of the line it is on func (c *Cursor) End() { c.X = util.CharacterCount(c.buf.LineBytes(c.Y)) - c.LastVisualX = c.GetVisualX() + c.StoreVisualX() } // CopySelection copies the user's selection to either "primary" diff --git a/internal/buffer/eventhandler.go b/internal/buffer/eventhandler.go index 10104f9c..e739f250 100644 --- a/internal/buffer/eventhandler.go +++ b/internal/buffer/eventhandler.go @@ -104,7 +104,7 @@ func (eh *EventHandler) DoTextEvent(t *TextEvent, useUndo bool) { c.OrigSelection[0] = move(c.OrigSelection[0]) c.OrigSelection[1] = move(c.OrigSelection[1]) c.Relocate() - c.LastVisualX = c.GetVisualX() + c.StoreVisualX() } if useUndo { diff --git a/runtime/plugins/comment/comment.lua b/runtime/plugins/comment/comment.lua index f86da945..ebb59626 100644 --- a/runtime/plugins/comment/comment.lua +++ b/runtime/plugins/comment/comment.lua @@ -107,7 +107,7 @@ function commentLine(bp, lineN, indentLen) bp.Cursor.Y = curpos.Y end bp.Cursor:Relocate() - bp.Cursor.LastVisualX = bp.Cursor:GetVisualX() + bp.Cursor:StoreVisualX() end function uncommentLine(bp, lineN, commentRegex) @@ -135,7 +135,7 @@ function uncommentLine(bp, lineN, commentRegex) end end bp.Cursor:Relocate() - bp.Cursor.LastVisualX = bp.Cursor:GetVisualX() + bp.Cursor:StoreVisualX() end function toggleCommentLine(bp, lineN, commentRegex) From 6214abba9a4e6512cf4090f408c7b1174918aee4 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 13 Oct 2024 23:12:04 +0200 Subject: [PATCH 2/4] Overhaul LastVisualX and GetVisualX() usage Restore the original meaning of LastVisualX before commit 6d13710d934d ("Implement moving cursor up/down within a wrapped line"): last visual x location of the cursor in a logical line in the buffer, not in a visual line on the screen (in other words, taking tabs and wide characters into account, but not taking softwrap into account). And add a separate LastWrappedVisualX field, similar to LastVisualX but taking softwrap into account as well. This allows tracking last x position at the same time for both cases when we care about softwrap and when we don't care about it. This can be useful, for example, for implementing cursor up/down movement actions that always move by logical lines, not by visual lines, even if softwrap is enabled (in addition to our default CursorUp and CursorDown actions that move by visual lines). Also this fixes a minor bug: in InsertTab(), when `tabstospaces` is enabled and we insert a tab, the amount of inserted spaces depends on the visual line wrapping (i.e. on the window width), which is probably not a good idea. --- internal/action/actions.go | 19 ++++++++++++------- internal/buffer/cursor.go | 17 ++++++++++++----- internal/display/bufwindow.go | 6 +++--- runtime/plugins/status/status.lua | 2 +- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index ccfb686b..2ea8f472 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -170,10 +170,10 @@ func (h *BufPane) MoveCursorUp(n int) { if sloc == vloc.SLoc { // we are at the beginning of buffer h.Cursor.Loc = h.Buf.Start() - h.Cursor.LastVisualX = 0 + h.Cursor.StoreVisualX() } else { vloc.SLoc = sloc - vloc.VisualX = h.Cursor.LastVisualX + vloc.VisualX = h.Cursor.LastWrappedVisualX h.Cursor.Loc = h.LocFromVLoc(vloc) } } @@ -189,11 +189,10 @@ func (h *BufPane) MoveCursorDown(n int) { if sloc == vloc.SLoc { // we are at the end of buffer h.Cursor.Loc = h.Buf.End() - vloc = h.VLocFromLoc(h.Cursor.Loc) - h.Cursor.LastVisualX = vloc.VisualX + h.Cursor.StoreVisualX() } else { vloc.SLoc = sloc - vloc.VisualX = h.Cursor.LastVisualX + vloc.VisualX = h.Cursor.LastWrappedVisualX h.Cursor.Loc = h.LocFromVLoc(vloc) } } @@ -889,7 +888,7 @@ func (h *BufPane) InsertTab() bool { b := h.Buf indent := b.IndentString(util.IntOpt(b.Settings["tabsize"])) tabBytes := len(indent) - bytesUntilIndent := tabBytes - (h.Cursor.GetVisualX() % tabBytes) + bytesUntilIndent := tabBytes - (h.Cursor.GetVisualX(false) % tabBytes) b.Insert(h.Cursor.Loc, indent[:bytesUntilIndent]) h.Relocate() return true @@ -1275,6 +1274,7 @@ func (h *BufPane) Copy() bool { func (h *BufPane) CopyLine() bool { origLoc := h.Cursor.Loc origLastVisualX := h.Cursor.LastVisualX + origLastWrappedVisualX := h.Cursor.LastWrappedVisualX origSelection := h.Cursor.CurSelection nlines := h.selectLines() @@ -1291,6 +1291,7 @@ func (h *BufPane) CopyLine() bool { h.Cursor.Loc = origLoc h.Cursor.LastVisualX = origLastVisualX + h.Cursor.LastWrappedVisualX = origLastWrappedVisualX h.Cursor.CurSelection = origSelection h.Relocate() return true @@ -1360,6 +1361,7 @@ func (h *BufPane) DuplicateLine() bool { if h.Cursor.HasSelection() { origLoc := h.Cursor.Loc origLastVisualX := h.Cursor.LastVisualX + origLastWrappedVisualX := h.Cursor.LastWrappedVisualX origSelection := h.Cursor.CurSelection start := h.Cursor.CurSelection[0] @@ -1380,6 +1382,7 @@ func (h *BufPane) DuplicateLine() bool { h.Cursor.Loc = origLoc h.Cursor.LastVisualX = origLastVisualX + h.Cursor.LastWrappedVisualX = origLastWrappedVisualX h.Cursor.CurSelection = origSelection if start.Y < end.Y { @@ -2070,6 +2073,7 @@ func (h *BufPane) SpawnMultiCursorUpN(n int) bool { c = buffer.NewCursor(h.Buf, buffer.Loc{lastC.X, lastC.Y - n}) c.LastVisualX = lastC.LastVisualX + c.LastWrappedVisualX = lastC.LastWrappedVisualX c.X = c.GetCharPosInLine(h.Buf.LineBytes(c.Y), c.LastVisualX) c.Relocate() } else { @@ -2082,9 +2086,10 @@ func (h *BufPane) SpawnMultiCursorUpN(n int) bool { h.Buf.DeselectCursors() vloc.SLoc = sloc - vloc.VisualX = lastC.LastVisualX + vloc.VisualX = lastC.LastWrappedVisualX c = buffer.NewCursor(h.Buf, h.LocFromVLoc(vloc)) c.LastVisualX = lastC.LastVisualX + c.LastWrappedVisualX = lastC.LastWrappedVisualX } h.Buf.AddCursor(c) diff --git a/internal/buffer/cursor.go b/internal/buffer/cursor.go index 784f2d39..007fcbe5 100644 --- a/internal/buffer/cursor.go +++ b/internal/buffer/cursor.go @@ -20,8 +20,14 @@ type Cursor struct { buf *Buffer Loc - // Last cursor x position + // Last visual x position of the cursor. Used in cursor up/down movements + // for remembering the original x position when moving to a line that is + // shorter than current x position. LastVisualX int + // Similar to LastVisualX but takes softwrapping into account, i.e. last + // visual x position in a visual (wrapped) line on the screen, which may be + // different from the line in the buffer. + LastWrappedVisualX int // The current selection as a range of character numbers (inclusive) CurSelection [2]Loc @@ -61,7 +67,7 @@ func (c *Cursor) Buf() *Buffer { // Goto puts the cursor at the given cursor's location and gives // the current cursor its selection too func (c *Cursor) Goto(b Cursor) { - c.X, c.Y, c.LastVisualX = b.X, b.Y, b.LastVisualX + c.X, c.Y, c.LastVisualX, c.LastWrappedVisualX = b.X, b.Y, b.LastVisualX, b.LastWrappedVisualX c.OrigSelection, c.CurSelection = b.OrigSelection, b.CurSelection } @@ -73,8 +79,8 @@ func (c *Cursor) GotoLoc(l Loc) { } // GetVisualX returns the x value of the cursor in visual spaces -func (c *Cursor) GetVisualX() int { - if c.buf.GetVisualX != nil { +func (c *Cursor) GetVisualX(wrap bool) int { + if wrap && c.buf.GetVisualX != nil { return c.buf.GetVisualX(c.Loc) } @@ -615,5 +621,6 @@ func (c *Cursor) RuneUnder(x int) rune { } func (c *Cursor) StoreVisualX() { - c.LastVisualX = c.GetVisualX() + c.LastVisualX = c.GetVisualX(false) + c.LastWrappedVisualX = c.GetVisualX(true) } diff --git a/internal/display/bufwindow.go b/internal/display/bufwindow.go index 1bfc04fd..6315bcc6 100644 --- a/internal/display/bufwindow.go +++ b/internal/display/bufwindow.go @@ -58,7 +58,7 @@ func (w *BufWindow) SetBuffer(b *buffer.Buffer) { if option == "softwrap" || option == "wordwrap" { w.Relocate() for _, c := range w.Buf.GetCursors() { - c.LastVisualX = c.GetVisualX() + c.LastWrappedVisualX = c.GetVisualX(true) } } } @@ -160,7 +160,7 @@ func (w *BufWindow) updateDisplayInfo() { if w.bufWidth != prevBufWidth && w.Buf.Settings["softwrap"].(bool) { for _, c := range w.Buf.GetCursors() { - c.LastVisualX = c.GetVisualX() + c.LastWrappedVisualX = c.GetVisualX(true) } } } @@ -238,7 +238,7 @@ func (w *BufWindow) Relocate() bool { // horizontal relocation (scrolling) if !b.Settings["softwrap"].(bool) { - cx := activeC.GetVisualX() + cx := activeC.GetVisualX(false) rw := runewidth.RuneWidth(activeC.RuneUnder(activeC.X)) if rw == 0 { rw = 1 // tab or newline diff --git a/runtime/plugins/status/status.lua b/runtime/plugins/status/status.lua index 4942d396..ae1f77a6 100644 --- a/runtime/plugins/status/status.lua +++ b/runtime/plugins/status/status.lua @@ -21,7 +21,7 @@ function lines(b) end function vcol(b) - return tostring(b:GetActiveCursor():GetVisualX()) + return tostring(b:GetActiveCursor():GetVisualX(false)) end function bytes(b) From 134cd999c60b84c16c7d68b2ea629897b626a49a Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 13 Oct 2024 23:14:37 +0200 Subject: [PATCH 3/4] Reset LastVisualX on undo/redo In cursor's Goto(), which is currently only used by undo and redo, we restore remembered LastVisualX and LastWrappedVisualX values. But if the window had been resized in the meantime, the LastWrappedVisualX may not be valid anymore. So it may cause the cursor moving to unexpected locations. So for simplicity just reset these values on undo or redo, instead of using remembered ones. --- internal/buffer/cursor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/buffer/cursor.go b/internal/buffer/cursor.go index 007fcbe5..d849f836 100644 --- a/internal/buffer/cursor.go +++ b/internal/buffer/cursor.go @@ -67,8 +67,9 @@ func (c *Cursor) Buf() *Buffer { // Goto puts the cursor at the given cursor's location and gives // the current cursor its selection too func (c *Cursor) Goto(b Cursor) { - c.X, c.Y, c.LastVisualX, c.LastWrappedVisualX = b.X, b.Y, b.LastVisualX, b.LastWrappedVisualX + c.X, c.Y = b.X, b.Y c.OrigSelection, c.CurSelection = b.OrigSelection, b.CurSelection + c.StoreVisualX() } // GotoLoc puts the cursor at the given cursor's location and gives From e6ed161ca48d7b5f2f8f1f4000e6de79729a4a13 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 13 Oct 2024 23:20:57 +0200 Subject: [PATCH 4/4] SpawnMultiCursorUp/Down: revert honoring softwrap Commit 9fdea8254250 ("Fix various issues with `SpawnMultiCursor{Up,Down}`") changed SpawnMultiCursorUp/Down actions to honor softwrap, i.e. spawn cursor in the next visual line within a wrapped line, which may not be the next logical line in a buffer. That was done for "consistency" with cursor movement actions CursorUp/Down etc. But it seems there are no actual use cases for that, whereas at least some users prefer spawning multicursor in the next logical line regardless of the softwrap setting. So restore the old behavior. Fixes #3499 --- internal/action/actions.go | 43 ++++++++++++-------------------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index 2ea8f472..54982957 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -2060,37 +2060,20 @@ func (h *BufPane) SpawnCursorAtLoc(loc buffer.Loc) *buffer.Cursor { // SpawnMultiCursorUpN is not an action func (h *BufPane) SpawnMultiCursorUpN(n int) bool { lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1) - var c *buffer.Cursor - if !h.Buf.Settings["softwrap"].(bool) { - if n > 0 && lastC.Y == 0 { - return false - } - if n < 0 && lastC.Y+1 == h.Buf.LinesNum() { - return false - } - - h.Buf.DeselectCursors() - - c = buffer.NewCursor(h.Buf, buffer.Loc{lastC.X, lastC.Y - n}) - c.LastVisualX = lastC.LastVisualX - c.LastWrappedVisualX = lastC.LastWrappedVisualX - c.X = c.GetCharPosInLine(h.Buf.LineBytes(c.Y), c.LastVisualX) - c.Relocate() - } else { - vloc := h.VLocFromLoc(lastC.Loc) - sloc := h.Scroll(vloc.SLoc, -n) - if sloc == vloc.SLoc { - return false - } - - h.Buf.DeselectCursors() - - vloc.SLoc = sloc - vloc.VisualX = lastC.LastWrappedVisualX - c = buffer.NewCursor(h.Buf, h.LocFromVLoc(vloc)) - c.LastVisualX = lastC.LastVisualX - c.LastWrappedVisualX = lastC.LastWrappedVisualX + if n > 0 && lastC.Y == 0 { + return false } + if n < 0 && lastC.Y+1 == h.Buf.LinesNum() { + return false + } + + h.Buf.DeselectCursors() + + c := buffer.NewCursor(h.Buf, buffer.Loc{lastC.X, lastC.Y - n}) + c.LastVisualX = lastC.LastVisualX + c.LastWrappedVisualX = lastC.LastWrappedVisualX + c.X = c.GetCharPosInLine(h.Buf.LineBytes(c.Y), c.LastVisualX) + c.Relocate() h.Buf.AddCursor(c) h.Buf.SetCurCursor(h.Buf.NumCursors() - 1)