Revert "Only make cursor visible after buffer or selection change"

This is currently broken on various corner cases and breaks the
"master branch should be good for day to day work" implicit rule,
ongoing work to stabilize this feature will take place on the
no-cursor-move-on-scroll branch until its deemed ready.

This reverts commit 1e38045d70.

Closes #4963

src/context.hh

@@ -144,7 +144,6 @@ public:
     void repeat_last_select() { if (m_last_select) m_last_select(*this); }
 
     Buffer* last_buffer() const;
-
 private:
     void begin_edition();
     void end_edition();
@@ -216,16 +215,9 @@ struct ScopedEdition
     ScopedEdition(Context& context)
         : m_context{context},
           m_buffer{context.has_buffer() ? &context.buffer() : nullptr}
-    {
+    { if (m_buffer) m_context.begin_edition(); }
-        if (m_buffer)
-            m_context.begin_edition();
-    }
 
-    ~ScopedEdition()
+    ~ScopedEdition() { if (m_buffer) m_context.end_edition(); }
-    {
-        if (m_buffer)
-            m_context.end_edition();
-    }
 
     Context& context() const { return m_context; }
 private:
@@ -238,19 +230,11 @@ struct ScopedSelectionEdition
     ScopedSelectionEdition(Context& context)
         : m_context{context},
           m_buffer{not (m_context.flags() & Context::Flags::Draft) and context.has_buffer() ? &context.buffer() : nullptr}
-    { 
+    { if (m_buffer) m_context.m_selection_history.begin_edition(); }
-        if (m_buffer)
-            m_context.m_selection_history.begin_edition();
-    }
-
     ScopedSelectionEdition(ScopedSelectionEdition&& other) : m_context{other.m_context}, m_buffer{other.m_buffer}
     { other.m_buffer = nullptr; }
 
-    ~ScopedSelectionEdition()
+    ~ScopedSelectionEdition() { if (m_buffer) m_context.m_selection_history.end_edition(); }
-    {
-        if (m_buffer)
-            m_context.m_selection_history.end_edition();
-    }
 private:
     Context& m_context;
     SafePtr<Buffer> m_buffer;

src/highlighter.hh

@@ -46,7 +46,6 @@ struct DisplaySetup
     DisplayCoord cursor_pos;
     // Offset of line and columns that must remain visible around cursor
     DisplayCoord scroll_offset;
-    bool ensure_cursor_visible;
 };
 
 using HighlighterIdList = ConstArrayView<StringView>;

src/highlighters.cc

@@ -752,8 +752,7 @@ struct WrapHighlighter : Highlighter
             win_line += wrap_count + 1;
 
             // scroll window to keep cursor visible, and update range as lines gets removed
-            while (setup.ensure_cursor_visible and
+            while (buf_line >= cursor.line and setup.first_line < cursor.line and
-                   buf_line >= cursor.line and setup.first_line < cursor.line and
                    setup.cursor_pos.line + setup.scroll_offset.line >= win_height)
             {
                 auto remove_count = 1 + line_wrap_count(setup.first_line, indent);
@@ -1661,8 +1660,7 @@ private:
                 setup.cursor_pos.column += cursor_move;
             }
 
-            if (setup.ensure_cursor_visible and
+            if (last.line >= setup.first_line and
-                last.line >= setup.first_line and
                 range.first.line <= setup.first_line + setup.line_count and
                 range.first.line != last.line)
             {

src/input_handler.cc

@@ -1870,27 +1870,25 @@ void scroll_window(Context& context, LineCount offset, bool mouse_dragging)
 
     win_pos.line = clamp(win_pos.line + offset, 0_line, line_count-1);
 
+    ScopedSelectionEdition selection_edition{context};
+    SelectionList& selections = context.selections();
+    Selection& main_selection = selections.main();
+    const BufferCoord anchor = main_selection.anchor();
+    const BufferCoordAndTarget cursor = main_selection.cursor();
+
+    auto cursor_off = mouse_dragging ? win_pos.line - window.position().line : 0;
+
+    auto line = clamp(cursor.line + cursor_off, win_pos.line + scrolloff.line,
+                      win_pos.line + win_dim.line - 1 - scrolloff.line);
+
+    const ColumnCount tabstop = context.options()["tabstop"].get<int>();
+    auto new_cursor = buffer.offset_coord(cursor, line - cursor.line, tabstop);
+    BufferCoord new_anchor = (mouse_dragging or new_cursor == cursor) ? anchor : new_cursor;
+
     window.set_position(win_pos);
-    if (mouse_dragging)
+    main_selection = { new_anchor, new_cursor };
-    {
-        ScopedSelectionEdition selection_edition{context};
-        SelectionList& selections = context.selections();
-        Selection& main_selection = selections.main();
-        const BufferCoord anchor = main_selection.anchor();
-        const BufferCoordAndTarget cursor = main_selection.cursor();
 
-        auto cursor_off = win_pos.line - window.position().line;
+    selections.sort_and_merge_overlapping();
-
-        auto line = clamp(cursor.line + cursor_off, win_pos.line + scrolloff.line,
-                          win_pos.line + win_dim.line - 1 - scrolloff.line);
-
-        const ColumnCount tabstop = context.options()["tabstop"].get<int>();
-        auto new_cursor = buffer.offset_coord(cursor, line - cursor.line, tabstop);
-
-        main_selection = { anchor, new_cursor };
-
-        selections.sort_and_merge_overlapping();
-    }
 }
 
 }

src/normal.cc

@@ -848,7 +848,7 @@ void regex_prompt(Context& context, String prompt, char reg, T func)
                 {
                     selections.update();
                     context.selections_write_only() = selections;
-                    if (context.has_window() and event != PromptEvent::Validate)
+                    if (context.has_window())
                         context.window().set_position(position);
 
                     context.input_handler().set_prompt_face(context.faces()["Prompt"]);

src/window.cc

@@ -85,34 +85,37 @@ static uint32_t compute_faces_hash(const FaceRegistry& faces)
 
 Window::Setup Window::build_setup(const Context& context) const
 {
-    return {m_position, m_dimensions,
+    Vector<BufferRange, MemoryDomain::Display> selections;
-            context.buffer().timestamp(),
+    for (auto& sel : context.selections())
-            compute_faces_hash(context.faces()),
+        selections.push_back({sel.cursor(), sel.anchor()});
-            context.selections().main_index(),
+
-            context.selections() | gather<Vector<BasicSelection, MemoryDomain::Display>>()};
+    return { m_position, m_dimensions,
+             context.buffer().timestamp(),
+             compute_faces_hash(context.faces()),
+             context.selections().main_index(),
+             std::move(selections) };
 }
 
 bool Window::needs_redraw(const Context& context) const
 {
     auto& selections = context.selections();
-    return m_position != m_last_setup.position or
+
+    if (m_position != m_last_setup.position or
         m_dimensions != m_last_setup.dimensions or
         context.buffer().timestamp() != m_last_setup.timestamp or
         selections.main_index() != m_last_setup.main_selection or
         selections.size() != m_last_setup.selections.size() or
-        compute_faces_hash(context.faces()) != m_last_setup.faces_hash or
+        compute_faces_hash(context.faces()) != m_last_setup.faces_hash)
-        not std::equal(selections.begin(), selections.end(),
+        return true;
-                       m_last_setup.selections.begin(), m_last_setup.selections.end());
-}
 
-bool Window::should_make_cursor_visible(const Context& context) const
+    for (int i = 0; i < selections.size(); ++i)
-{
+    {
-    auto& selections = context.selections();
+        if (selections[i].cursor() != m_last_setup.selections[i].begin or
-    return context.buffer().timestamp() != m_last_setup.timestamp or
+            selections[i].anchor() != m_last_setup.selections[i].end)
-        selections.main_index() != m_last_setup.main_selection or
+            return true;
-        selections.size() != m_last_setup.selections.size() or
+    }
-        not std::equal(selections.begin(), selections.end(),
+
-                       m_last_setup.selections.begin(), m_last_setup.selections.end());
+    return false;
 }
 
 const DisplayBuffer& Window::update_display_buffer(const Context& context)
@@ -212,15 +215,11 @@ DisplaySetup Window::compute_display_setup(const Context& context) const
     const int tabstop = context.options()["tabstop"].get<int>();
     const auto& cursor = context.selections().main().cursor();
 
-    bool ensure_cursor_visible = should_make_cursor_visible(context);
+    // Ensure cursor line is visible
-
+    if (cursor.line - offset.line < win_pos.line)
-    if (ensure_cursor_visible)
+        win_pos.line = std::max(0_line, cursor.line - offset.line);
-    {
+    if (cursor.line + offset.line >= win_pos.line + m_dimensions.line)
-        if (cursor.line - offset.line < win_pos.line)
+        win_pos.line = std::min(buffer().line_count()-1, cursor.line + offset.line - m_dimensions.line + 1);
-            win_pos.line = std::max(0_line, cursor.line - offset.line);
-        if (cursor.line + offset.line >= win_pos.line + m_dimensions.line)
-            win_pos.line = std::min(buffer().line_count()-1, cursor.line + offset.line - m_dimensions.line + 1);
-    }
 
     DisplaySetup setup{
         win_pos.line,
@@ -229,15 +228,13 @@ DisplaySetup Window::compute_display_setup(const Context& context) const
         0_col,
         {cursor.line - win_pos.line,
          get_column(buffer(), tabstop, cursor) - win_pos.column},
-        offset,
+        offset
-        ensure_cursor_visible
     };
     for (auto pass : { HighlightPass::Move, HighlightPass::Wrap })
         m_builtin_highlighters.compute_display_setup({context, setup, pass, {}}, setup);
     check_display_setup(setup, *this);
 
     // now ensure the cursor column is visible
-    if (ensure_cursor_visible)
     {
         auto underflow = std::max(-setup.first_column,
                                   setup.cursor_pos.column - setup.scroll_offset.column);

src/window.hh

@@ -43,7 +43,7 @@ public:
     Buffer& buffer() const { return *m_buffer; }
 
     bool needs_redraw(const Context& context) const;
-    void force_redraw() { m_last_setup.dimensions = {}; }
+    void force_redraw() { m_last_setup = Setup{}; }
 
     void set_client(Client* client) { m_client = client; }
 
@@ -60,8 +60,6 @@ private:
     void run_hook_in_own_context(Hook hook, StringView param,
                                  String client_name = "");
 
-    bool should_make_cursor_visible(const Context& context) const;
-
     SafePtr<Buffer> m_buffer;
     SafePtr<Client> m_client;
 
@@ -79,7 +77,7 @@ private:
         size_t timestamp;
         size_t faces_hash;
         size_t main_selection;
-        Vector<BasicSelection, MemoryDomain::Display> selections;
+        Vector<BufferRange, MemoryDomain::Display> selections;
     };
     Setup build_setup(const Context& context) const;
     Setup m_last_setup;

test/regression/3219-scroll-json-ui/script

@@ -6,8 +6,8 @@ ui_out '{ "jsonrpc": "2.0", "method": "draw_status", "params": [[], [{ "face": {
 ui_out '{ "jsonrpc": "2.0", "method": "set_cursor", "params": ["buffer", { "line": 0, "column": 0 }] }'
 ui_out '{ "jsonrpc": "2.0", "method": "refresh", "params": [true] }'
 ui_in '{ "jsonrpc": "2.0", "method": "scroll", "params": [ 2 ] }'
-ui_out '{ "jsonrpc": "2.0", "method": "draw", "params": [[[{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "03\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "04\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "05\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "06\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "07\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "08\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "09\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "10\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "11\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "12\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "13\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "14\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "15\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "16\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "17\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "18\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "19\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "20\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "21\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "22\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "23\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "24\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "25\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "26\u000a" }]], { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, { "fg": "blue", "bg": "default", "underline": "default", "attributes": [] }] }'
+ui_out '{ "jsonrpc": "2.0", "method": "draw", "params": [[[{ "face": { "fg": "black", "bg": "white", "underline": "default", "attributes": [] }, "contents": "0" }, { "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "3\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "04\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "05\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "06\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "07\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "08\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "09\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "10\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "11\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "12\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "13\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "14\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "15\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "16\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "17\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "18\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "19\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "20\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "21\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "22\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "23\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "24\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "25\u000a" }], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "26\u000a" }]], { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, { "fg": "blue", "bg": "default", "underline": "default", "attributes": [] }] }'
 ui_out '{ "jsonrpc": "2.0", "method": "info_hide", "params": [] }'
-ui_out '{ "jsonrpc": "2.0", "method": "draw_status", "params": [[], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "out 1:1 " }, { "face": { "fg": "black", "bg": "yellow", "underline": "default", "attributes": [] }, "contents": "" }, { "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "blue", "bg": "default", "underline": "default", "attributes": [] }, "contents": "1 sel" }, { "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": " - client0@[kak-tests]" }], { "fg": "cyan", "bg": "default", "underline": "default", "attributes": [] }] }'
+ui_out '{ "jsonrpc": "2.0", "method": "draw_status", "params": [[], [{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "out 3:1 " }, { "face": { "fg": "black", "bg": "yellow", "underline": "default", "attributes": [] }, "contents": "" }, { "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "blue", "bg": "default", "underline": "default", "attributes": [] }, "contents": "1 sel" }, { "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": " - client0@[kak-tests]" }], { "fg": "cyan", "bg": "default", "underline": "default", "attributes": [] }] }'
 ui_out '{ "jsonrpc": "2.0", "method": "set_cursor", "params": ["buffer", { "line": 0, "column": 0 }] }'
 ui_out '{ "jsonrpc": "2.0", "method": "refresh", "params": [false] }'

test/regression/4839-scroll-invalid-cursor/cmd

@@ -1 +1 @@
-l<c-l><pagedown>
+l<pagedown>

test/regression/4839-scroll-invalid-cursor/script

@@ -1,2 +1,2 @@
-ui_out -ignore 7
+ui_out -ignore 1
-ui_out '{ "jsonrpc": "2.0", "method": "draw", "params": [[[{ "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "😃\u000a" }]], { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, { "fg": "blue", "bg": "default", "underline": "default", "attributes": [] }] }'
+ui_out '{ "jsonrpc": "2.0", "method": "draw", "params": [[[{ "face": { "fg": "black", "bg": "white", "underline": "default", "attributes": [] }, "contents": "😃" }, { "face": { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, "contents": "\u000a" }]], { "fg": "default", "bg": "default", "underline": "default", "attributes": [] }, { "fg": "blue", "bg": "default", "underline": "default", "attributes": [] }] }'