Buffer: move back Modification as an implementation detail

* Filters now only works on insertion and take directly the
   iterator and content
 * use Buffer::insert and Buffer::erase to modify a buffer

src/buffer.cc

@@ -31,7 +31,7 @@ Buffer::Buffer(String name, Type type,
 {
     BufferManager::instance().register_buffer(*this);
     if (not initial_content.empty())
-        apply_modification(Modification::make_insert(begin(), std::move(initial_content)));
+        do_insert(begin(), std::move(initial_content));
 
     Editor editor_for_hooks(*this);
     Context context(editor_for_hooks);
@@ -160,7 +160,19 @@ void Buffer::end_undo_group()
     m_current_undo_group.clear();
 }
 
-Modification Modification::inverse() const
+// A Modification holds a single atomic modification to Buffer
+struct Buffer::Modification
+{
+    enum Type { Insert, Erase };
+
+    Type           type;
+    BufferIterator position;
+    String         content;
+
+    Modification(Type type, BufferIterator position, String content)
+        : type(type), position(position), content(std::move(content)) {}
+
+    Modification inverse() const
     {
         Type inverse_type;
         switch (type)
@@ -171,6 +183,7 @@ Modification Modification::inverse() const
         }
         return Modification(inverse_type, position, content);
     }
+};
 
 bool Buffer::undo()
 {
@@ -214,7 +227,7 @@ void Buffer::check_invariant() const
     }
 }
 
-void Buffer::insert(const BufferIterator& pos, const String& content)
+void Buffer::do_insert(const BufferIterator& pos, const String& content)
 {
     BufferSize offset = pos.offset();
 
@@ -288,7 +301,7 @@ void Buffer::insert(const BufferIterator& pos, const String& content)
         listener->on_insert(begin_it, end_it);
 }
 
-void Buffer::erase(const BufferIterator& pos, BufferSize length)
+void Buffer::do_erase(const BufferIterator& pos, BufferSize length)
 {
     BufferIterator end = pos + length;
     assert(end.is_valid());
@@ -320,7 +333,7 @@ void Buffer::apply_modification(const Modification& modification)
     {
         BufferIterator pos = modification.position < end() ?
                              modification.position : end();
-        insert(pos, modification.content);
+        do_insert(pos, modification.content);
         break;
     }
     case Modification::Erase:
@@ -328,7 +341,7 @@ void Buffer::apply_modification(const Modification& modification)
         size_t count = modification.content.length();
         assert(string(modification.position, modification.position + count)
                == modification.content);
-        erase(modification.position, count);
+        do_erase(modification.position, count);
         break;
     }
     default:
@@ -336,13 +349,22 @@ void Buffer::apply_modification(const Modification& modification)
     }
 }
 
-void Buffer::modify(Modification&& modification)
+void Buffer::insert(const BufferIterator& pos, const String& content)
 {
-    if (modification.content.empty())
+    if (content.empty())
+        return;
+    m_current_undo_group.emplace_back(Modification::Insert, pos, content);
+    do_insert(pos, content);
+}
+
+void Buffer::erase(const BufferIterator& begin, const BufferIterator& end)
+{
+    if (begin == end)
         return;
 
-    apply_modification(modification);
+    m_current_undo_group.emplace_back(Modification::Erase, begin,
-    m_current_undo_group.push_back(std::move(modification));
+                                      string(begin, end));
+    do_erase(begin, end - begin);
 }
 
 Window* Buffer::get_or_create_window()

src/buffer.hh

@@ -14,7 +14,6 @@ namespace Kakoune
 {
 
 class Buffer;
-class Modification;
 class Window;
 
 typedef int       BufferPos;
@@ -82,24 +81,6 @@ private:
     friend class Buffer;
 };
 
-// A Modification holds a single atomic modification to Buffer
-struct Modification
-{
-    enum Type { Insert, Erase };
-
-    Type           type;
-    BufferIterator position;
-    String         content;
-
-    Modification(Type type, BufferIterator position, String content)
-        : type(type), position(position), content(std::move(content)) {}
-
-    Modification inverse() const;
-
-    static Modification make_erase(BufferIterator begin, BufferIterator end);
-    static Modification make_insert(BufferIterator position, String content);
-};
-
 class BufferChangeListener
 {
 public:
@@ -130,8 +111,8 @@ public:
 
     Type type() const { return m_type; }
 
-    // apply given modification to buffer.
+    void insert(const BufferIterator& pos, const String& content);
-    void           modify(Modification&& modification);
+    void erase(const BufferIterator& begin, const BufferIterator& end);
 
     void           begin_undo_group();
     void           end_undo_group();
@@ -198,8 +179,8 @@ private:
     };
     std::vector<Line> m_lines;
 
-    void insert(const BufferIterator& pos, const String& content);
+    void do_insert(const BufferIterator& pos, const String& content);
-    void erase(const BufferIterator& pos, BufferSize length);
+    void do_erase(const BufferIterator& pos, BufferSize length);
 
     BufferPos line_at(const BufferIterator& iterator) const;
     BufferSize line_length(BufferPos line) const;
@@ -207,6 +188,7 @@ private:
     String  m_name;
     const Type   m_type;
 
+    struct Modification;
     typedef std::vector<Modification> UndoGroup;
 
     std::vector<UndoGroup>           m_history;
@@ -226,18 +208,6 @@ private:
     HookManager   m_hook_manager;
 };
 
-inline Modification Modification::make_erase(BufferIterator begin,
-                                             BufferIterator end)
-{
-    return Modification(Erase, begin, begin.buffer().string(begin, end));
-}
-
-inline Modification Modification::make_insert(BufferIterator position,
-                                              String content)
-{
-    return Modification(Insert, position, std::move(content));
-}
-
 }
 
 #include "buffer_iterator.inl.hh"

src/editor.cc

@@ -22,7 +22,7 @@ void Editor::erase()
 {
     scoped_edition edition(*this);
     for (auto& sel : selections())
-        m_buffer.modify(Modification::make_erase(sel.begin(), sel.end()));
+        m_buffer.erase(sel.begin(), sel.end());
 }
 
 template<bool append>
@@ -32,7 +32,7 @@ static void do_insert(Editor& editor, const String& string)
     for (auto& sel : editor.selections())
     {
         BufferIterator pos = append ? sel.end() : sel.begin();
-        editor.buffer().modify(Modification::make_insert(pos, string));
+        editor.buffer().insert(pos, string);
     }
 }
 
@@ -48,7 +48,7 @@ static void do_insert(Editor& editor, const memoryview<String>& strings)
         BufferIterator pos = append ? editor.selections()[i].end()
                                     : editor.selections()[i].begin();
         size_t index = std::min(i, strings.size()-1);
-        editor.buffer().modify(Modification::make_insert(pos, strings[index]));
+        editor.buffer().insert(pos, strings[index]);
     }
 }
 
@@ -348,9 +348,9 @@ IncrementalInserter::IncrementalInserter(Editor& editor, Mode mode)
         }
         sel = Selection(first, last);
 
-        if (mode == Mode::OpenLineBelow or mode == Mode::OpenLineAbove)
-            apply(Modification::make_insert(sel.last(), "\n"));
     }
+    if (mode == Mode::OpenLineBelow or mode == Mode::OpenLineAbove)
+        insert("\n");
 }
 
 IncrementalInserter::~IncrementalInserter()
@@ -362,16 +362,16 @@ IncrementalInserter::~IncrementalInserter()
     m_editor.on_incremental_insertion_end();
 }
 
-void IncrementalInserter::apply(Modification&& modification) const
-{
-    m_editor.filters()(m_editor.buffer(), modification);
-    m_editor.buffer().modify(std::move(modification));
-}
-
 void IncrementalInserter::insert(const String& string)
 {
+    Buffer& buffer = m_editor.buffer();
     for (auto& sel : m_editor.selections())
-        apply(Modification::make_insert(sel.last(), string));
+    {
+        BufferIterator position = sel.last();
+        String content = string;
+        m_editor.filters()(buffer, position, content);
+        m_editor.buffer().insert(position, content);
+    }
 }
 
 void IncrementalInserter::insert(const memoryview<String>& strings)
@@ -384,7 +384,7 @@ void IncrementalInserter::erase()
     for (auto& sel : m_editor.m_selections.back())
     {
         BufferIterator pos = sel.last();
-        apply(Modification::make_erase(pos-1, pos));
+        m_editor.buffer().erase(pos-1, pos);
     }
 }
 

src/editor.hh

@@ -119,8 +119,6 @@ public:
     Buffer& buffer() const { return m_editor.buffer(); }
 
 private:
-    void apply(Modification&& modification) const;
-
     Mode    m_mode;
     Editor& m_editor;
     scoped_edition m_edition;

src/file.cc

@@ -117,7 +117,7 @@ Buffer* create_buffer_from_file(const String& filename)
                 if (buf[pos] == '\r')
                     crlf = true;
 
-                buffer->modify(Modification::make_insert(buffer->end(), String(buf+start, buf+pos)));
+                buffer->insert(buffer->end(), String(buf+start, buf+pos));
                 start = pos+1;
             }
             ++pos;

src/filter.hh

@@ -8,13 +8,13 @@ namespace Kakoune
 {
 
 class Buffer;
-class Modification;
+class BufferIterator;
 
 // A Filter is a function which is applied to a Buffer and a pending
 // Modification in order to mutate the Buffer or the Modification
 // prior to it's application.
 
-typedef std::function<void (Buffer& buffer, Modification& modification)> FilterFunc;
+typedef std::function<void (Buffer& buffer, BufferIterator& position, String& content)> FilterFunc;
 typedef std::pair<String, FilterFunc> FilterAndId;
 
 }

src/filter_group.cc

@@ -6,10 +6,11 @@
 namespace Kakoune
 {
 
-void FilterGroup::operator()(Buffer& buffer, Modification& modification)
+void FilterGroup::operator()(Buffer& buffer,
+                             BufferIterator& position, String& content)
 {
     for (auto& filter : m_filters)
-       filter.second(buffer, modification);
+       filter.second(buffer, position, content);
 }
 
 void FilterGroup::append(FilterAndId&& filter)

src/filter_group.hh

@@ -12,7 +12,7 @@ namespace Kakoune
 class FilterGroup
 {
 public:
-    void operator()(Buffer& buffer, Modification& modification);
+    void operator()(Buffer& buffer, BufferIterator& position, String& content);
 
     void append(FilterAndId&& filter);
     void remove(const String& id);

src/filters.cc

@@ -6,48 +6,46 @@
 namespace Kakoune
 {
 
-void preserve_indent(Buffer& buffer, Modification& modification)
+void preserve_indent(Buffer& buffer, BufferIterator& position, String& content)
 {
-    if (modification.type == Modification::Insert and
+    if (content == "\n")
-        modification.content == "\n")
     {
-         BufferIterator line_begin = buffer.iterator_at_line_begin(modification.position - 1);
+         BufferIterator line_begin = buffer.iterator_at_line_begin(position - 1);
          BufferIterator first_non_white = line_begin;
          while ((*first_non_white == '\t' or *first_non_white == ' ') and
                 not first_non_white.is_end())
              ++first_non_white;
 
-         modification.content += buffer.string(line_begin, first_non_white);
+         content += buffer.string(line_begin, first_non_white);
     }
 }
 
-void cleanup_whitespaces(Buffer& buffer, Modification& modification)
+void cleanup_whitespaces(Buffer& buffer, BufferIterator& position, String& content)
 {
-    if (modification.type == Modification::Insert and
+    if (content[0] == '\n' and not position.is_begin())
-        modification.content[0] == '\n' and not modification.position.is_begin())
     {
-        BufferIterator position = modification.position-1;
+        BufferIterator whitespace_start = position-1;
-        while ((*position == ' ' or *position == '\t') and not position.is_begin())
+        while ((*whitespace_start == ' ' or *whitespace_start == '\t') and
-            --position;
+               not whitespace_start .is_begin())
-        ++position;
+            --whitespace_start;
-        if (position != modification.position)
+        ++whitespace_start;
+        if (whitespace_start!= position)
         {
-            buffer.modify(Modification::make_erase(position, modification.position));
+            buffer.erase(whitespace_start, position);
-            modification.position = position;
+            position = whitespace_start;
         }
     }
 }
 
-void expand_tabulations(Buffer& buffer, Modification& modification)
+void expand_tabulations(Buffer& buffer, BufferIterator& position, String& content)
 {
     const int tabstop = buffer.option_manager()["tabstop"].as_int();
-    if (modification.type == Modification::Insert and
+    if (content == "\t")
-        modification.content == "\t")
     {
         int column = 0;
-        BufferCoord pos = buffer.line_and_column_at(modification.position);
+        BufferCoord pos = buffer.line_and_column_at(position);
         for (auto line_it = buffer.iterator_at({pos.line, 0});
-             line_it != modification.position; ++line_it)
+             line_it != position; ++line_it)
         {
             assert(*line_it != '\n');
             if (*line_it == '\t')
@@ -57,13 +55,13 @@ void expand_tabulations(Buffer& buffer, Modification& modification)
         }
 
         int count = tabstop - (column % tabstop);
-        modification.content = String();
+        content = String();
         for (int i = 0; i < count; ++i)
-            modification.content += ' ';
+            content += ' ';
     }
 }
 
-template<void (*filter_func)(Buffer&, Modification&)>
+template<void (*filter_func)(Buffer&, BufferIterator&, String&)>
 class SimpleFilterFactory
 {
 public:

src/unit_tests.cc

@@ -20,7 +20,7 @@ void test_buffer()
     assert(buffer.line_and_column_at(i) == BufferCoord{0 COMMA 6});
     ++i;
     assert(buffer.line_and_column_at(i) == BufferCoord{1 COMMA 0});
-    buffer.modify(Modification::make_insert(i, "tchou kanaky\n"));
+    buffer.insert(i, "tchou kanaky\n");
     assert(buffer.line_count() == 5);
 
     BufferIterator begin = buffer.iterator_at({ 4, 1 });
@@ -41,7 +41,7 @@ void test_editor()
     for (auto& sel : editor.selections())
     {
         assert(*sel.begin() == '\n');
-        editor.buffer().modify(Modification::make_erase(sel.begin(), sel.end()));
+        editor.buffer().erase(sel.begin(), sel.end());
     }
 }