remove ModificationListener and use a list of iterators to update instead

This permits to fix a bug in BufferIterator::upgrade, replaced by
BufferIterator::on_insert and BufferIterator::on_erase. ModificationListener
was only used to updating iterators anyway.

src/buffer.cc

@@ -43,7 +43,7 @@ Buffer::~Buffer()
 {
     m_windows.clear();
     BufferManager::instance().unregister_buffer(this);
-    assert(m_modification_listeners.empty());
+    assert(m_iterators_to_update.empty());
 }
 
 BufferIterator Buffer::iterator_at(const BufferCoord& line_and_column) const
@@ -204,6 +204,7 @@ void Buffer::insert(const BufferIterator& pos, const String& content)
     for (size_t i = pos.line()+1; i < line_count(); ++i)
         m_lines[i].start += content.length();
 
+    BufferCoord end_pos;
     // if we inserted at the end of the buffer, we may have created a new
     // line without inserting a '\n'
     if (pos == end() and (pos == begin() or *(pos-1) == '\n'))
@@ -219,6 +220,8 @@ void Buffer::insert(const BufferIterator& pos, const String& content)
         }
         if (start != content.length())
             m_lines.push_back({ offset + start, content.substr(start) });
+
+        end_pos = end().m_coord;
     }
     else
     {
@@ -249,12 +252,17 @@ void Buffer::insert(const BufferIterator& pos, const String& content)
             }
         }
         if (start == 0)
-            m_lines.insert(line_it, { offset + start - (int)prefix.length(), prefix + content + suffix });
+            line_it = m_lines.insert(line_it, { offset + start - (int)prefix.length(), prefix + content + suffix });
         else
-            m_lines.insert(line_it, { offset + start, content.substr(start) + suffix });
+            line_it = m_lines.insert(line_it, { offset + start, content.substr(start) + suffix });
+
+        end_pos = { int(line_it - m_lines.begin()), int(line_it->length() - suffix.length()) };
     }
 
     check_invariant();
+
+    for (auto iterator : m_iterators_to_update)
+        iterator->on_insert(pos.m_coord, end_pos);
 }
 
 void Buffer::erase(const BufferIterator& pos, BufferSize length)
@@ -272,6 +280,9 @@ void Buffer::erase(const BufferIterator& pos, BufferSize length)
         m_lines[i].start -= length;
 
     check_invariant();
+
+    for (auto iterator : m_iterators_to_update)
+        iterator->on_erase(pos.m_coord, end.m_coord);
 }
 
 void Buffer::apply_modification(const Modification& modification)
@@ -299,9 +310,6 @@ void Buffer::apply_modification(const Modification& modification)
     default:
         assert(false);
     }
-
-    for (auto listener : m_modification_listeners)
-        listener->on_modification(modification);
 }
 
 void Buffer::modify(Modification&& modification)
@@ -342,21 +350,19 @@ void Buffer::notify_saved()
     m_last_save_undo_index = history_cursor_index;
 }
 
-void Buffer::register_modification_listener(ModificationListener* listener)
+void Buffer::add_iterator_to_update(BufferIterator& iterator)
 {
-    assert(listener);
-    assert(not contains(m_modification_listeners, listener));
-    m_modification_listeners.push_back(listener);
+    assert(not contains(m_iterators_to_update, &iterator));
+    m_iterators_to_update.push_back(&iterator);
 }
 
-void Buffer::unregister_modification_listener(ModificationListener* listener)
+void Buffer::remove_iterator_from_update(BufferIterator& iterator)
 {
-    assert(listener);
-    auto it = std::find(m_modification_listeners.begin(),
-                        m_modification_listeners.end(),
-                        listener);
-    assert(it != m_modification_listeners.end());
-    m_modification_listeners.erase(it);
+    auto it = std::find(m_iterators_to_update.begin(),
+                        m_iterators_to_update.end(),
+                        &iterator);
+    assert(it != m_iterators_to_update.end());
+    m_iterators_to_update.erase(it);
 }
 
 }

src/buffer.hh

@@ -41,7 +41,7 @@ public:
     typedef const value_type& reference;
     typedef std::bidirectional_iterator_tag iterator_category;
 
-    BufferIterator() : m_buffer(NULL) {}
+    BufferIterator() : m_buffer(nullptr) {}
     BufferIterator(const Buffer& buffer, BufferCoord coord);
     BufferIterator& operator=(const BufferIterator& iterator);
 
@@ -68,7 +68,8 @@ public:
     bool is_end() const;
     bool is_valid() const;
 
-    void update(const Modification& modification);
+    void on_insert(const BufferCoord& begin, const BufferCoord& end);
+    void on_erase(const BufferCoord& begin, const BufferCoord& end);
 
     const Buffer& buffer() const;
 
@@ -101,12 +102,6 @@ struct Modification
                                     const String& content);
 };
 
-class ModificationListener
-{
-public:
-    virtual void on_modification(const Modification& modification) = 0;
-};
-
 // A Buffer is a in-memory representation of a file
 //
 // The Buffer class permits to read and mutate this file
@@ -160,8 +155,8 @@ public:
     Type type() const { return m_type; }
     void notify_saved();
 
-    void register_modification_listener(ModificationListener* listener);
-    void unregister_modification_listener(ModificationListener* listener);
+    void add_iterator_to_update(BufferIterator& iterator);
+    void remove_iterator_from_update(BufferIterator& iterator);
 
     // returns an iterator pointing to the first character of the line
     // iterator is on
@@ -212,7 +207,7 @@ private:
 
     size_t m_last_save_undo_index;
 
-    std::vector<ModificationListener*> m_modification_listeners;
+    std::vector<BufferIterator*> m_iterators_to_update;
 
     OptionManager m_option_manager;
 };

src/buffer_iterator.inl.hh

@@ -71,67 +71,45 @@ inline bool BufferIterator::operator>=(const BufferIterator& iterator) const
     return (m_coord >= iterator.m_coord);
 }
 
-inline BufferCoord measure_string(const String& string)
+inline void BufferIterator::on_insert(const BufferCoord& begin,
+                                      const BufferCoord& end)
 {
-    BufferCoord res;
-    for (auto c : string)
-    {
-        if (c == '\n')
-        {
-            ++res.line;
-            res.column = 0;
-        }
-        else
-           ++res.column;
-    }
-    return res;
-}
-
-inline BufferCoord advance(const BufferCoord& base, const BufferCoord& offset)
-{
-    if (offset.line == 0)
-        return BufferCoord{base.line, base.column + offset.column};
-    else
-        return BufferCoord{base.line + offset.line, offset.column};
-}
-
-inline void BufferIterator::update(const Modification& modification)
-{
-    const BufferIterator& pos = modification.position;
-    if (*this < pos)
+    if (m_coord < begin)
         return;
 
-    BufferCoord measure = measure_string(modification.content);
-    if (modification.type == Modification::Erase)
-    {
-        BufferCoord end = advance(pos.m_coord, measure);
-        if (m_coord <= end)
-            m_coord = pos.m_coord;
-        else
-        {
-            m_coord.line -= measure.line;
-            if (measure.line > 0 and pos.line() == m_coord.line)
-                m_coord.column += pos.column();
-            if (end.line == m_coord.line)
-                m_coord.column -= measure.column;
-        }
+    if (begin.line == line())
+        m_coord.column = end.column + m_coord.column - begin.column;
+    m_coord.line += end.line - begin.line;
 
-        if (is_end())
-            operator--();
-    }
-    else
-    {
-        assert(modification.type == Modification::Insert);
-        if (pos.line() == line())
-        {
-            BufferCoord end = advance(pos.m_coord, measure);
-            m_coord.column = end.column + column() - pos.column();
-        }
-        m_coord.line += measure.line;
-    }
     assert(is_valid());
 }
 
+inline void BufferIterator::on_erase(const BufferCoord& begin,
+                                     const BufferCoord& end)
+{
+    if (m_coord < begin)
+        return;
+
+    BufferCoord measure;
+    measure.line = end.line - begin.line;
+    measure.column = end.column - (measure.line == 0 ? begin.column : 0);
+    if (m_coord <= end)
+        m_coord = begin;
+    else
+    {
+        m_coord.line -= measure.line;
+        if (measure.line > 0 and begin.line == m_coord.line)
+            m_coord.column += begin.column;
+        if (end.line == m_coord.line)
+            m_coord.column -= measure.column;
+    }
+
+    if (is_end())
+        operator--();
+    assert(is_valid());
+}
+
+
 inline BufferChar BufferIterator::operator*() const
 {
     assert(m_buffer);

src/selection.cc

@@ -54,20 +54,18 @@ void Selection::merge_with(const Selection& selection)
     m_last = selection.m_last;
 }
 
-void Selection::on_modification(const Modification& modification)
-{
-    m_first.update(modification);
-    m_last.update(modification);
-}
-
 void Selection::register_with_buffer()
 {
-    const_cast<Buffer&>(m_first.buffer()).register_modification_listener(this);
+    Buffer& buffer = const_cast<Buffer&>(m_first.buffer());
+    buffer.add_iterator_to_update(m_first);
+    buffer.add_iterator_to_update(m_last);
 }
 
 void Selection::unregister_with_buffer()
 {
-    const_cast<Buffer&>(m_first.buffer()).unregister_modification_listener(this);
+    Buffer& buffer = const_cast<Buffer&>(m_first.buffer());
+    buffer.remove_iterator_from_update(m_first);
+    buffer.remove_iterator_from_update(m_last);
 }
 
 }

src/selection.hh

@@ -12,7 +12,7 @@ namespace Kakoune
 // Selections are oriented, first may be > last, and inclusive.
 // Selection updates it's iterators according to modifications made
 // in the buffer.
-struct Selection : public ModificationListener
+struct Selection
 {
     Selection(const BufferIterator& first, const BufferIterator& last);
     Selection(const Selection& other);
@@ -34,8 +34,6 @@ private:
     BufferIterator m_first;
     BufferIterator m_last;
 
-    void on_modification(const Modification& modification);
-
     void register_with_buffer();
     void unregister_with_buffer();
 };