Cleanup replaced range selection logic

Do not access Buffer::m_changes to find the inserted range, return
it directly from Buffer::insert and Buffer::replace. This fixes a
wrong behaviour where replacing at eof would lose the selected end
of line (as the implementation does not actually replace that end
of line)

src/buffer.cc

@@ -444,12 +444,12 @@ void Buffer::check_invariant() const
 #endif
 }
 
-BufferCoord Buffer::do_insert(BufferCoord pos, StringView content)
+BufferRange Buffer::do_insert(BufferCoord pos, StringView content)
 {
     kak_assert(is_valid(pos));
 
     if (content.empty())
-        return pos;
+        return {pos, pos};
 
     const bool at_end = is_end(pos);
     const bool append_lines = at_end and (m_lines.empty() or byte_at(back_coord()) == '\n');
@@ -489,7 +489,7 @@ BufferCoord Buffer::do_insert(BufferCoord pos, StringView content)
                             : BufferCoord{ last_line, m_lines[last_line].length() - suffix.length() };
 
     m_changes.push_back({ Change::Insert, pos, end });
-    return end;
+    return {pos, end};
 }
 
 BufferCoord Buffer::do_erase(BufferCoord begin, BufferCoord end)
@@ -543,13 +543,13 @@ void Buffer::apply_modification(const Modification& modification)
     }
 }
 
-BufferCoord Buffer::insert(BufferCoord pos, StringView content)
+BufferRange Buffer::insert(BufferCoord pos, StringView content)
 {
     throw_if_read_only();
 
     kak_assert(is_valid(pos));
     if (content.empty())
-        return pos;
+        return {pos, pos};
 
     StringDataPtr real_content;
     if (is_end(pos) and content.back() != '\n')
@@ -584,21 +584,20 @@ BufferCoord Buffer::erase(BufferCoord begin, BufferCoord end)
     return do_erase(begin, end);
 }
 
-BufferCoord Buffer::replace(BufferCoord begin, BufferCoord end, StringView content)
+BufferRange Buffer::replace(BufferCoord begin, BufferCoord end, StringView content)
 {
     throw_if_read_only();
 
+    if (std::equal(iterator_at(begin), iterator_at(end), content.begin(), content.end()))
+        return {begin, end};
+
     if (is_end(end) and not content.empty() and content.back() == '\n')
     {
-        end = back_coord();
+        auto pos = insert(erase(begin, back_coord()),
-        content = content.substr(0, content.length() - 1);
+                          content.substr(0, content.length() - 1)).begin;
+        return {pos, end_coord()};
     }
-
+    return insert(erase(begin, end), content);
-    if (std::equal(iterator_at(begin), iterator_at(end), content.begin(), content.end()))
-        return begin;
-
-    auto pos = erase(begin, end);
-    return insert(pos, content);
 }
 
 bool Buffer::is_modified() const

src/buffer.hh

@@ -7,6 +7,7 @@
 #include "enum.hh"
 #include "file.hh"
 #include "optional.hh"
+#include "range.hh"
 #include "safe_ptr.hh"
 #include "scope.hh"
 #include "shared_string.hh"
@@ -103,6 +104,7 @@ private:
 };
 
 using BufferLines = Vector<StringDataPtr, MemoryDomain::BufferContent>;
+using BufferRange = Range<BufferCoord>;
 
 // A Buffer is a in-memory representation of a file
 //
@@ -139,9 +141,9 @@ public:
     bool set_name(String name);
     void update_display_name();
 
-    BufferCoord insert(BufferCoord pos, StringView content);
+    BufferRange insert(BufferCoord pos, StringView content);
     BufferCoord erase(BufferCoord begin, BufferCoord end);
-    BufferCoord replace(BufferCoord begin, BufferCoord end, StringView content);
+    BufferRange replace(BufferCoord begin, BufferCoord end, StringView content);
 
     size_t          timestamp() const;
     void            set_fs_status(FsStatus);
@@ -231,7 +233,7 @@ public:
 private:
     void on_option_changed(const Option& option) override;
 
-    BufferCoord do_insert(BufferCoord pos, StringView content);
+    BufferRange do_insert(BufferCoord pos, StringView content);
     BufferCoord do_erase(BufferCoord begin, BufferCoord end);
 
     struct Modification;

src/buffer_utils.hh

@@ -20,7 +20,7 @@ inline BufferCoord erase(Buffer& buffer, const Selection& range)
     return buffer.erase(range.min(), buffer.char_next(range.max()));
 }
 
-inline BufferCoord replace(Buffer& buffer, const Selection& range, StringView content)
+inline BufferRange replace(Buffer& buffer, const Selection& range, StringView content)
 {
     return buffer.replace(range.min(), buffer.char_next(range.max()), content);
 }

src/normal.cc

@@ -524,7 +524,7 @@ BufferCoord apply_diff(Buffer& buffer, BufferCoord pos, StringView before, Strin
             break;
         case DiffOp::Add:
             pos = buffer.insert(pos, {lines_after[posB].begin(),
-                                      lines_after[posB + len - 1].end()});
+                                      lines_after[posB + len - 1].end()}).end;
             break;
         case DiffOp::Remove:
             pos = buffer.erase(pos, buffer.advance(pos, byte_count(lines_before, posA, len)));

src/selection.cc

@@ -406,39 +406,25 @@ void SelectionList::insert(ConstArrayView<String> strings, InsertMode mode,
         const auto pos = (mode == InsertMode::Replace) ?
             sel.min() : changes_tracker.get_new_coord(insert_pos[index]);
 
-        if (mode == InsertMode::Replace)
-            replace(*m_buffer, sel, str);
-        else
-            m_buffer->insert(pos, str);
-
-        size_t old_timestamp = m_timestamp;
-        changes_tracker.update(*m_buffer, m_timestamp);
-
-        if (out_insert_pos)
-            out_insert_pos->push_back(pos);
-
         if (mode == InsertMode::Replace)
         {
-            auto changes = m_buffer->changes_since(old_timestamp);
+            auto range = replace(*m_buffer, sel, str);
-            if (changes.size() == 1) // Nothing got inserted, either str was empty, or just \n at end of buffer
-                sel.anchor() = sel.cursor() = m_buffer->clamp(pos);
-            else if (changes.size() == 2)
-            {
             // we want min and max from *before* we do any change
             auto& min = sel.min();
             auto& max = sel.max();
-                min = changes.back().begin;
+            min = range.begin;
-                max = m_buffer->char_prev(changes.back().end);
+            max = m_buffer->char_prev(range.end);
         }
         else
-                kak_assert(changes.empty());
-        }
-        else if (not str.empty())
         {
-            auto& change = m_buffer->changes_since(0).back();
+            auto range = m_buffer->insert(pos, str);
-            sel.anchor() = m_buffer->clamp(update_insert(sel.anchor(), change.begin, change.end));
+            sel.anchor() = m_buffer->clamp(update_insert(sel.anchor(), range.begin, range.end));
-            sel.cursor() = m_buffer->clamp(update_insert(sel.cursor(), change.begin, change.end));
+            sel.cursor() = m_buffer->clamp(update_insert(sel.cursor(), range.begin, range.end));
         }
+
+        changes_tracker.update(*m_buffer, m_timestamp);
+        if (out_insert_pos)
+            out_insert_pos->push_back(pos);
     }
 
     // We might just have been deleting text if strings were empty,

test/regression/0-rotate-at-eof-mutates-selection/cmd

@@ -0,0 +1 @@
+%<a-s><a-)><a-)>

test/regression/0-rotate-at-eof-mutates-selection/in

@@ -0,0 +1,3 @@
+foo
+bar
+baz

test/regression/0-rotate-at-eof-mutates-selection/out

@@ -0,0 +1,3 @@
+bar
+baz
+foo