rework selection insert/replace using a for_each method

expose that method so that various commands can take advantage of
it for performance or simplicity purposes.

src/buffer_utils.hh

@@ -20,11 +20,6 @@ inline BufferCoord erase(Buffer& buffer, const Selection& range)
     return buffer.erase(range.min(), buffer.char_next(range.max()));
 }
 
-inline BufferRange replace(Buffer& buffer, const Selection& range, StringView content)
-{
-    return buffer.replace(range.min(), buffer.char_next(range.max()), content);
-}
-
 void replace(Buffer& buffer, ArrayView<BufferRange> ranges, ConstArrayView<String> strings);
 
 inline CharCount char_length(const Buffer& buffer, const Selection& range)

src/normal.cc

@@ -413,11 +413,10 @@ void replace_with_char(Context& context, NormalParams)
             return;
         ScopedEdition edition(context);
         Buffer& buffer = context.buffer();
-        SelectionList& selections = context.selections();
+        context.selections().for_each([&](size_t index, Selection& sel) {
-        selections.insert([&](size_t index, BufferCoord) {
+            CharCount count = char_length(buffer, sel);
-            CharCount count = char_length(buffer, selections[index]);
+            replace(buffer, sel, String{*cp, count});
-            return String{*cp, count};
+        });
-        }, InsertMode::Replace);
     }, "replace with char", "enter char to replace with\n");
 }
 
@@ -434,17 +433,15 @@ void for_each_codepoint(Context& context, NormalParams)
 
     ScopedEdition edition(context);
     Buffer& buffer = context.buffer();
-    SelectionList& selections = context.selections();
 
-    selections.insert([&](size_t index, BufferCoord) {
+    context.selections().for_each([&](size_t index, Selection& sel) {
-        auto& sel = selections[index];
         String str;
         for (auto begin = Utf8It{buffer.iterator_at(sel.min()), buffer},
                   end = Utf8It{buffer.iterator_at(sel.max()), buffer}+1;
              begin != end; ++begin)
             utf8::dump(std::back_inserter(str), func(*begin));
-        return str;
+        replace(buffer, sel, str);
-    }, InsertMode::Replace);
+    });
 }
 
 void command(const Context& context, EnvVarMap env_vars, char reg = 0)
@@ -642,15 +639,17 @@ void insert_output(Context& context, NormalParams params)
 
             ScopedEdition edition(context);
             auto& selections = context.selections();
+            auto& buffer = context.buffer();
             const size_t old_main = selections.main_index();
 
-            selections.insert([&](size_t index, BufferCoord) {
+            selections.for_each([&](size_t index, Selection& sel) {
                 selections.set_main_index(index);
                 auto [out, status] = ShellManager::instance().eval(
-                    cmdline, context, content(context.buffer(), selections[index]),
+                    cmdline, context, content(context.buffer(), sel),
                     ShellManager::Flags::WaitForStdout);
-                return out;
+
-            }, mode);
+                insert(buffer, sel, out, mode);
+            });
 
             selections.set_main_index(old_main);
         });
@@ -688,15 +687,19 @@ void change(Context& context, NormalParams params)
     enter_insert_mode<InsertMode::Replace>(context, params);
 }
 
-InsertMode adapt_for_linewise(InsertMode mode)
+InsertMode adapt_for_linewise(InsertMode mode, bool linewise)
 {
+    if (not linewise)
+        return mode;
+
     switch (mode)
     {
         case InsertMode::Append: return InsertMode::InsertAtNextLineBegin;
         case InsertMode::Insert: return InsertMode::InsertAtLineBegin;
-        case InsertMode::Replace: return InsertMode::Replace;
+        default: break;
-        default: return InsertMode::Insert;
     }
+    kak_assert(false);
+    return InsertMode::Insert;
 }
 
 template<InsertMode mode>
@@ -707,10 +710,12 @@ void paste(Context& context, NormalParams params)
     const bool linewise = any_of(strings, [](StringView str) {
         return not str.empty() and str.back() == '\n';
     });
-    const auto effective_mode = linewise ? adapt_for_linewise(mode) : mode;
 
     ScopedEdition edition(context);
-    context.selections().insert(strings, effective_mode);
+    if (mode == InsertMode::Replace)
+        context.selections().replace(strings);
+    else
+        context.selections().insert(strings, adapt_for_linewise(mode, linewise));
 }
 
 template<InsertMode mode>
@@ -718,7 +723,7 @@ void paste_all(Context& context, NormalParams params)
 {
     const char reg = params.reg ? params.reg : '"';
     auto strings = RegisterManager::instance()[reg].get(context);
-    InsertMode effective_mode = mode;
+    bool linewise = false;
     String all;
     Vector<ByteCount> offsets;
     for (auto& str : strings)
@@ -727,27 +732,25 @@ void paste_all(Context& context, NormalParams params)
             continue;
 
         if (str.back() == '\n')
-            effective_mode = adapt_for_linewise(mode);
+            linewise = true;
         all += str;
         offsets.push_back(all.length());
     }
 
-    Vector<BufferCoord> insert_pos;
+    InsertMode effective_mode = adapt_for_linewise(mode, linewise);
+
+    Buffer& buffer = context.buffer();
+    Vector<Selection> result;
     auto& selections = context.selections();
     {
         ScopedEdition edition(context);
-        selections.insert([&](size_t, BufferCoord pos) {
+        selections.for_each([&](size_t, const Selection& sel) {
-            insert_pos.push_back(pos);
+            auto range = (mode == InsertMode::Replace) ?
-            return String::no_copy(all);
+                buffer.replace(sel.min(), buffer.char_next(sel.max()), all)
-        }, effective_mode);
+              : buffer.insert(get_insert_pos(buffer, sel, effective_mode), all);
-    }
 
-    const Buffer& buffer = context.buffer();
-    Vector<Selection> result;
-    for (auto& ins_pos : insert_pos)
-    {
             ByteCount pos_offset = 0;
-        BufferCoord pos = ins_pos;
+            BufferCoord pos = range.begin;
             for (auto offset : offsets)
             {
                 BufferCoord end = buffer.advance(pos, offset - pos_offset - 1);
@@ -755,8 +758,8 @@ void paste_all(Context& context, NormalParams params)
                 pos = buffer.next(end);
                 pos_offset = offset;
             }
+        });
     }
-    if (not result.empty())
     selections = std::move(result);
 }
 
@@ -1079,7 +1082,7 @@ void join_lines_select_spaces(Context& context, NormalParams)
         return;
     context.selections_write_only() = std::move(selections);
     ScopedEdition edition(context);
-    context.selections().insert(" "_str, InsertMode::Replace);
+    context.selections().replace({" "_str});
 }
 
 void join_lines(Context& context, NormalParams params)
@@ -1478,7 +1481,7 @@ void rotate_selections_content(Context& context, NormalParams params)
             main = main < new_beg ? end - (new_beg - main) : it + (main - new_beg);
         it = end;
     }
-    selections.insert(strings, InsertMode::Replace);
+    selections.replace(strings);
     selections.set_main_index(main - strings.begin());
 }
 
@@ -1696,7 +1699,7 @@ void tabs_to_spaces(Context& context, NormalParams params)
         }
     }
     if (not tabs.empty())
-        SelectionList{ buffer, std::move(tabs) }.insert(spaces, InsertMode::Replace);
+        SelectionList{ buffer, std::move(tabs) }.replace(spaces);
 }
 
 void spaces_to_tabs(Context& context, NormalParams params)
@@ -1732,7 +1735,7 @@ void spaces_to_tabs(Context& context, NormalParams params)
         }
     }
     if (not spaces.empty())
-        SelectionList{ buffer, std::move(spaces) }.insert("\t"_str, InsertMode::Replace);
+        SelectionList{ buffer, std::move(spaces) }.replace("\t"_str);
 }
 
 void trim_selections(Context& context, NormalParams)

src/selection.cc

@@ -367,7 +367,7 @@ BufferCoord get_insert_pos(const Buffer& buffer, const Selection& sel,
     case InsertMode::AppendAtLineEnd:
         return {sel.max().line, buffer[sel.max().line].length() - 1};
     case InsertMode::InsertAtNextLineBegin:
-        return sel.max().line+1;
+        return std::min(buffer.line_count(), sel.max().line+1);
     default:
         kak_assert(false);
         return {};
@@ -390,22 +390,15 @@ void SelectionList::insert(ConstArrayView<String> strings, InsertMode mode)
     if (strings.empty())
         return;
 
-    insert([&](size_t index, BufferCoord) {
+    for_each([&](size_t index, Selection& sel) {
-        return String::no_copy(strings[std::min(strings.size()-1, index)]);
+        Kakoune::insert(*m_buffer, sel, strings[std::min(strings.size()-1, index)], mode);
-    }, mode);
+    });
 }
 
-void SelectionList::insert(ContentFunc get_content, InsertMode mode)
+void SelectionList::for_each(ApplyFunc func)
 {
     update();
 
-    Vector<BufferCoord> insert_pos;
-    if (mode != InsertMode::Replace)
-    {
-        for (auto& sel : m_selections)
-            insert_pos.push_back(get_insert_pos(*m_buffer, sel, mode));
-    }
-
     ForwardChangesTracker changes_tracker;
     for (size_t index = 0; index < m_selections.size(); ++index)
     {
@@ -413,42 +406,49 @@ void SelectionList::insert(ContentFunc get_content, InsertMode mode)
 
         sel.anchor() = changes_tracker.get_new_coord_tolerant(sel.anchor());
         sel.cursor() = changes_tracker.get_new_coord_tolerant(sel.cursor());
-        kak_assert(m_buffer->is_valid(sel.anchor()) and
+        kak_assert(m_buffer->is_valid(sel.anchor()) and m_buffer->is_valid(sel.cursor()));
-                   m_buffer->is_valid(sel.cursor()));
 
-        const auto pos = (mode == InsertMode::Replace) ?
+        func(index, sel);
-            sel.min() : changes_tracker.get_new_coord(insert_pos[index]);
-
-        String str = get_content(index, pos);
-
-        if (mode == InsertMode::Replace)
-        {
-            auto range = replace(*m_buffer, sel, str);
-            // we want min and max from *before* we do any change
-            auto& min = sel.min();
-            auto& max = sel.max();
-            min = range.begin;
-            max = range.end > range.begin ? m_buffer->char_prev(range.end) : range.begin;
-        }
-        else
-        {
-            auto range = m_buffer->insert(pos, str);
-            sel.anchor() = m_buffer->clamp(update_insert(sel.anchor(), range.begin, range.end));
-            sel.cursor() = m_buffer->clamp(update_insert(sel.cursor(), range.begin, range.end));
-        }
 
         changes_tracker.update(*m_buffer, m_timestamp);
     }
 
     // We might just have been deleting text if strings were empty,
     // in which case we could have some selections pushed out of the buffer
-    if (mode == InsertMode::Replace)
     fix_overflowing_selections(m_selections, *m_buffer);
 
     check_invariant();
     m_buffer->check_invariant();
 }
 
+
+void replace(Buffer& buffer, Selection& sel, StringView content)
+{
+    // we want min and max from *before* we do any change
+    auto& min = sel.min();
+    auto& max = sel.max();
+    BufferRange range = buffer.replace(min, buffer.char_next(max), content);
+    min = range.begin;
+    max = range.end > range.begin ? buffer.char_prev(range.end) : range.begin;
+}
+
+void insert(Buffer& buffer, Selection& sel, StringView content, InsertMode mode)
+{
+    auto range = buffer.insert(get_insert_pos(buffer, sel, mode), content);
+    sel.anchor() = buffer.clamp(update_insert(sel.anchor(), range.begin, range.end));
+    sel.cursor() = buffer.clamp(update_insert(sel.cursor(), range.begin, range.end));
+}
+
+void SelectionList::replace(ConstArrayView<String> strings)
+{
+    if (strings.empty())
+        return;
+
+    for_each([&](size_t index, Selection& sel) {
+        Kakoune::replace(*m_buffer, sel, strings[std::min(strings.size()-1, index)]);
+    });
+}
+
 void SelectionList::erase()
 {
     update();

src/selection.hh

@@ -87,6 +87,12 @@ enum class InsertMode : unsigned
     OpenLineAbove
 };
 
+BufferCoord get_insert_pos(const Buffer& buffer, const Selection& sel, InsertMode mode);
+
+void replace(Buffer& buffer, Selection& sel, StringView content);
+void insert(Buffer& buffer, Selection& sel, StringView content, InsertMode mode);
+
+
 struct SelectionList
 {
     static constexpr MemoryDomain Domain = MemoryDomain::Selections;
@@ -145,9 +151,12 @@ struct SelectionList
     size_t timestamp() const { return m_timestamp; }
     void force_timestamp(size_t timestamp) { m_timestamp = timestamp; }
 
+    using ApplyFunc = FunctionRef<void (size_t index, Selection& sel)>;
+    void for_each(ApplyFunc apply);
+
     void insert(ConstArrayView<String> strings, InsertMode mode);
-    using ContentFunc = FunctionRef<String (size_t index, BufferCoord insert_pos)>;
+    void replace(ConstArrayView<String> strings);
-    void insert(ContentFunc get_content, InsertMode mode);
+
     void erase();
 
 private: