#4134
Issues/zyedidia/micro/#4134

MoveLinesUp doesn't scroll the view to follow the selection when it moves above the viewport

Opendimk90opened 4mo ago

Description

When moving a multi-line selection up with MoveLinesUp (Alt+↑), the selected lines move correctly but the view does not scroll to follow them once the block moves above the top of the viewport - the moved text disappears off the top of the screen.

MoveLinesDown (Alt+↓) does not have this problem: the view scrolls to keep the block visible as it moves past the bottom edge.

Steps to reproduce

  1. Open a file longer than the terminal height.
  2. Scroll so the cursor/selection is near the top of the viewport.
  3. Select several lines (e.g. Shift+↓).
  4. Hold Alt+↑ to move the block upward past the top of the view.

Environment

  • Version: 2.0.15
  • OS: ArchLinux 7.0.14
  • Terminal: native linux terminal (no GUI)

Activity

14
molloyzak134mo ago

Had a look at this. Both MoveLinesUp and MoveLinesDown call h.Relocate() at the end of actions.go, so the scroll logic itself is symmetric — Relocate in bufwindow.go handles cursor-above-viewport and cursor-below-viewport the same way.

The asymmetry is upstream. After Buf.MoveLinesUp runs, the event handler auto-adjusts cursor positions for the insert/remove operations (eventhandler.go covers Loc, CurSelection, and OrigSelection for all cursors). But MoveLinesUp then applies a manual CurSelection[sel].Y -= 1 on top of that in the compensate path. MoveLinesDown skips that entirely.

What I think happens: that manual decrement lands CurSelection[sel].Y at a different spot than where the event handler puts Loc. Relocate reads activeC.Loc, not CurSelection, so if Loc thinks the cursor is still visible, it won't scroll. By the time the cursor is actually outside the viewport, the text has been off-screen for a few presses.

Might be as simple as dropping the manual adjustment and letting the event handler do its thing, or making sure the position Relocate reads matches the visual position of the moved block.

molloyzak134mo ago

Had a look at this. Both MoveLinesUp and MoveLinesDown call h.Relocate() at the end of actions.go, so the scroll logic itself is symmetric — Relocate in bufwindow.go handles cursor-above-viewport and cursor-below-viewport the same way.

The asymmetry is upstream. After Buf.MoveLinesUp runs, the event handler auto-adjusts cursor positions for the insert/remove operations (eventhandler.go covers Loc, CurSelection, and OrigSelection for all cursors). But MoveLinesUp then applies a manual CurSelection[sel].Y -= 1 on top of that in the compensate path. MoveLinesDown skips that entirely.

What I think happens: that manual decrement lands CurSelection[sel].Y at a different spot than where the event handler puts Loc. Relocate reads activeC.Loc, not CurSelection, so if Loc thinks the cursor is still visible, it won't scroll. By the time the cursor is actually outside the viewport, the text has been off-screen for a few presses.

Might be as simple as dropping the manual adjustment and letting the event handler do its thing, or making sure the position Relocate reads matches the visual position of the moved block.

molloyzak134mo ago

Had a look at this. Both MoveLinesUp and MoveLinesDown call h.Relocate() at the end of actions.go, so the scroll logic itself is symmetric — Relocate in bufwindow.go handles cursor-above-viewport and cursor-below-viewport the same way.

The asymmetry is upstream. After Buf.MoveLinesUp runs, the event handler auto-adjusts cursor positions for the insert/remove operations (eventhandler.go covers Loc, CurSelection, and OrigSelection for all cursors). But MoveLinesUp then applies a manual CurSelection[sel].Y -= 1 on top of that in the compensate path. MoveLinesDown skips that entirely.

What I think happens: that manual decrement lands CurSelection[sel].Y at a different spot than where the event handler puts Loc. Relocate reads activeC.Loc, not CurSelection, so if Loc thinks the cursor is still visible, it won't scroll. By the time the cursor is actually outside the viewport, the text has been off-screen for a few presses.

Might be as simple as dropping the manual adjustment and letting the event handler do its thing, or making sure the position Relocate reads matches the visual position of the moved block.

molloyzak134mo ago

Had a look at this. Both MoveLinesUp and MoveLinesDown call h.Relocate() at the end of actions.go, so the scroll logic itself is symmetric — Relocate in bufwindow.go handles cursor-above-viewport and cursor-below-viewport the same way.

The asymmetry is upstream. After Buf.MoveLinesUp runs, the event handler auto-adjusts cursor positions for the insert/remove operations (eventhandler.go covers Loc, CurSelection, and OrigSelection for all cursors). But MoveLinesUp then applies a manual CurSelection[sel].Y -= 1 on top of that in the compensate path. MoveLinesDown skips that entirely.

What I think happens: that manual decrement lands CurSelection[sel].Y at a different spot than where the event handler puts Loc. Relocate reads activeC.Loc, not CurSelection, so if Loc thinks the cursor is still visible, it won't scroll. By the time the cursor is actually outside the viewport, the text has been off-screen for a few presses.

Might be as simple as dropping the manual adjustment and letting the event handler do its thing, or making sure the position Relocate reads matches the visual position of the moved block.

dimk904mo ago

Temporary workaround via Lua plugin: re-points the cursor onto the moved block so Relocate() follows it.

Create ~/.config/micro/plug/movelines/movelines.lua:

VERSION = "1.0.0"

local buffer = import("micro/buffer")

function moveUp(bufpane)
    bufpane:MoveLinesUp()
    
    local cursor = bufpane.Cursor
    if cursor:HasSelection() then
        -- CurSelection entries are *Loc, so compare X/Y directly and build a value Loc for GotoLoc.
        local first = cursor.CurSelection[1]
        local second = cursor.CurSelection[2]
        local top = first
        if second.Y < first.Y or (second.Y == first.Y and second.X < first.X) then
            top = second
        end
        cursor:GotoLoc(buffer.Loc(top.X, top.Y))
    end
    
    bufpane:Relocate()
end

Bind Alt+↑ in ~/.config/micro/bindings.json:

{
    "AltUp": "lua:movelines.moveUp"
}
dimk904mo ago

Temporary workaround via Lua plugin: re-points the cursor onto the moved block so Relocate() follows it.

Create ~/.config/micro/plug/movelines/movelines.lua:

VERSION = "1.0.0"

local buffer = import("micro/buffer")

function moveUp(bufpane)
    bufpane:MoveLinesUp()
    
    local cursor = bufpane.Cursor
    if cursor:HasSelection() then
        -- CurSelection entries are *Loc, so compare X/Y directly and build a value Loc for GotoLoc.
        local first = cursor.CurSelection[1]
        local second = cursor.CurSelection[2]
        local top = first
        if second.Y < first.Y or (second.Y == first.Y and second.X < first.X) then
            top = second
        end
        cursor:GotoLoc(buffer.Loc(top.X, top.Y))
    end
    
    bufpane:Relocate()
end

Bind Alt+↑ in ~/.config/micro/bindings.json:

{
    "AltUp": "lua:movelines.moveUp"
}
dimk904mo ago

Temporary workaround via Lua plugin: re-points the cursor onto the moved block so Relocate() follows it.

Create ~/.config/micro/plug/movelines/movelines.lua:

VERSION = "1.0.0"

local buffer = import("micro/buffer")

function moveUp(bufpane)
    bufpane:MoveLinesUp()
    
    local cursor = bufpane.Cursor
    if cursor:HasSelection() then
        -- CurSelection entries are *Loc, so compare X/Y directly and build a value Loc for GotoLoc.
        local first = cursor.CurSelection[1]
        local second = cursor.CurSelection[2]
        local top = first
        if second.Y < first.Y or (second.Y == first.Y and second.X < first.X) then
            top = second
        end
        cursor:GotoLoc(buffer.Loc(top.X, top.Y))
    end
    
    bufpane:Relocate()
end

Bind Alt+↑ in ~/.config/micro/bindings.json:

{
    "AltUp": "lua:movelines.moveUp"
}
dimk904mo ago

Temporary workaround via Lua plugin: re-points the cursor onto the moved block so Relocate() follows it.

Create ~/.config/micro/plug/movelines/movelines.lua:

VERSION = "1.0.0"

local buffer = import("micro/buffer")

function moveUp(bufpane)
    bufpane:MoveLinesUp()
    
    local cursor = bufpane.Cursor
    if cursor:HasSelection() then
        -- CurSelection entries are *Loc, so compare X/Y directly and build a value Loc for GotoLoc.
        local first = cursor.CurSelection[1]
        local second = cursor.CurSelection[2]
        local top = first
        if second.Y < first.Y or (second.Y == first.Y and second.X < first.X) then
            top = second
        end
        cursor:GotoLoc(buffer.Loc(top.X, top.Y))
    end
    
    bufpane:Relocate()
end

Bind Alt+↑ in ~/.config/micro/bindings.json:

{
    "AltUp": "lua:movelines.moveUp"
}
JoeKar4mo ago

You could test with:

diff --git a/internal/action/actions.go b/internal/action/actions.go
index 491199f7..6d8b3323 100644
--- a/internal/action/actions.go
+++ b/internal/action/actions.go
@@ -1530,6 +1530,7 @@ func (h *BufPane) MoveLinesUp() bool {
                )
                if compensate {
                        h.Cursor.CurSelection[sel].Y -= 1
+                       h.Cursor.GotoLoc(buffer.Loc{h.Cursor.CurSelection[sel].X, h.Cursor.CurSelection[sel].Y-1})
                }
        } else {
                if h.Cursor.Loc.Y == 0 {
@@ -1561,14 +1562,20 @@ func (h *BufPane) MoveLinesDown() bool {
                        sel = 0
                }
 
+               compensate := false
                if h.Cursor.CurSelection[sel].X != 0 {
                        end++
+               } else {
+                       compensate = true
                }
 
                h.Buf.MoveLinesDown(
                        start,
                        end,
                )
+               if compensate {
+                       h.Cursor.GotoLoc(buffer.Loc{h.Cursor.CurSelection[sel].X, h.Cursor.CurSelection[sel].Y-1})
+               }
        } else {
                if h.Cursor.Loc.Y >= h.Buf.LinesNum()-1 {
                        InfoBar.Message("Cannot move further down")
JoeKar4mo ago

You could test with:

diff --git a/internal/action/actions.go b/internal/action/actions.go
index 491199f7..6d8b3323 100644
--- a/internal/action/actions.go
+++ b/internal/action/actions.go
@@ -1530,6 +1530,7 @@ func (h *BufPane) MoveLinesUp() bool {
                )
                if compensate {
                        h.Cursor.CurSelection[sel].Y -= 1
+                       h.Cursor.GotoLoc(buffer.Loc{h.Cursor.CurSelection[sel].X, h.Cursor.CurSelection[sel].Y-1})
                }
        } else {
                if h.Cursor.Loc.Y == 0 {
@@ -1561,14 +1562,20 @@ func (h *BufPane) MoveLinesDown() bool {
                        sel = 0
                }
 
+               compensate := false
                if h.Cursor.CurSelection[sel].X != 0 {
                        end++
+               } else {
+                       compensate = true
                }
 
                h.Buf.MoveLinesDown(
                        start,
                        end,
                )
+               if compensate {
+                       h.Cursor.GotoLoc(buffer.Loc{h.Cursor.CurSelection[sel].X, h.Cursor.CurSelection[sel].Y-1})
+               }
        } else {
                if h.Cursor.Loc.Y >= h.Buf.LinesNum()-1 {
                        InfoBar.Message("Cannot move further down")
JoeKar4mo ago

You could test with:

diff --git a/internal/action/actions.go b/internal/action/actions.go
index 491199f7..6d8b3323 100644
--- a/internal/action/actions.go
+++ b/internal/action/actions.go
@@ -1530,6 +1530,7 @@ func (h *BufPane) MoveLinesUp() bool {
                )
                if compensate {
                        h.Cursor.CurSelection[sel].Y -= 1
+                       h.Cursor.GotoLoc(buffer.Loc{h.Cursor.CurSelection[sel].X, h.Cursor.CurSelection[sel].Y-1})
                }
        } else {
                if h.Cursor.Loc.Y == 0 {
@@ -1561,14 +1562,20 @@ func (h *BufPane) MoveLinesDown() bool {
                        sel = 0
                }
 
+               compensate := false
                if h.Cursor.CurSelection[sel].X != 0 {
                        end++
+               } else {
+                       compensate = true
                }
 
                h.Buf.MoveLinesDown(
                        start,
                        end,
                )
+               if compensate {
+                       h.Cursor.GotoLoc(buffer.Loc{h.Cursor.CurSelection[sel].X, h.Cursor.CurSelection[sel].Y-1})
+               }
        } else {
                if h.Cursor.Loc.Y >= h.Buf.LinesNum()-1 {
                        InfoBar.Message("Cannot move further down")
dimk903mo ago

@JoeKar Thanks for the patch! It fixes the disappearing selection for blocks up to 4 lines - those now stay in view when moving up. With taller selections some of the moved lines still end up hidden above the top of the viewport.

dimk903mo ago

@JoeKar Thanks for the patch! It fixes the disappearing selection for blocks up to 4 lines - those now stay in view when moving up. With taller selections some of the moved lines still end up hidden above the top of the viewport.

JoeKar3mo ago

With taller selections some of the moved lines still end up hidden above the top of the viewport.

AFAIK this is more or less intended and depends on the selection direction (marked top-down or bottom-up) and scroll direction.