move selection updating code out of selection, to DynamicSelectionList

this avoids a lot of unnecessary (add|remove)_change_listener as
creating temporary Selections do not call that anymore.

Use can choose between a SelectionList which or a DynamicSelectionList
depending on wethear the buffer will be modified or not during the
selections lifetime.

src/context.hh

@@ -99,23 +99,21 @@ struct Context
 
     void push_jump()
     {
-        const Selection& jump = editor().selections().back();
+        const SelectionList& jump = editor().selections();
         if (m_current_jump != m_jump_list.end())
         {
             auto begin = m_current_jump;
-            // when jump overlaps with m_current_jump, we replace m_current_jump
-            // instead of pushing after it.
-            if (&begin->first().buffer() != &jump.first().buffer() or
-                not overlaps(*begin, jump))
+            if (&editor().buffer() != &begin->buffer() or
+                (const SelectionList&)(*begin) != jump)
                 ++begin;
             m_jump_list.erase(begin, m_jump_list.end());
         }
 
-        m_jump_list.push_back(jump);
+        m_jump_list.push_back({editor().buffer(), jump});
         m_current_jump = m_jump_list.end();
     }
 
-    Selection jump_forward()
+    const SelectionList& jump_forward()
     {
         if (m_current_jump != m_jump_list.end() and
             m_current_jump + 1 != m_jump_list.end())
@@ -123,7 +121,7 @@ struct Context
         throw runtime_error("no next jump");
     }
 
-    Selection jump_backward()
+    const SelectionList& jump_backward()
     {
         if (m_current_jump != m_jump_list.begin())
         {
@@ -141,7 +139,7 @@ struct Context
     {
         for (auto it = m_jump_list.begin(); it != m_jump_list.end();)
         {
-            if (&it->first().buffer() == &buffer)
+            if (&it->buffer() == &buffer)
             {
                 if (it < m_current_jump)
                     --m_current_jump;
@@ -164,8 +162,9 @@ private:
     Insertion m_last_insert = {InsertMode::Insert, {}};
     int m_numeric_param = 0;
 
-    SelectionList           m_jump_list;
-    SelectionList::iterator m_current_jump = m_jump_list.begin();
+    using JumpList = std::vector<DynamicSelectionList>;
+    JumpList           m_jump_list;
+    JumpList::iterator m_current_jump = m_jump_list.begin();
 };
 
 }

src/dynamic_selection_list.cc

@@ -0,0 +1,81 @@
+#include "dynamic_selection_list.hh"
+
+namespace Kakoune
+{
+
+DynamicSelectionList::DynamicSelectionList(const Buffer& buffer,
+                                           SelectionList selections)
+    : m_buffer(&buffer), m_selections(std::move(selections))
+{
+    m_buffer->add_change_listener(*this);
+}
+
+DynamicSelectionList::~DynamicSelectionList()
+{
+    m_buffer->remove_change_listener(*this);
+}
+
+DynamicSelectionList::DynamicSelectionList(const DynamicSelectionList& other)
+    : m_selections(other.m_selections), m_buffer(other.m_buffer)
+{
+    m_buffer->add_change_listener(*this);
+}
+
+DynamicSelectionList& DynamicSelectionList::operator=(const DynamicSelectionList& other)
+{
+    m_selections = other.m_selections;
+    if (m_buffer != other.m_buffer)
+    {
+        m_buffer->remove_change_listener(*this);
+        m_buffer = other.m_buffer;
+        m_buffer->add_change_listener(*this);
+    }
+    return *this;
+}
+
+DynamicSelectionList::DynamicSelectionList(DynamicSelectionList&& other)
+    : m_selections(std::move(other.m_selections)), m_buffer(other.m_buffer)
+{
+    m_buffer->add_change_listener(*this);
+}
+
+DynamicSelectionList& DynamicSelectionList::operator=(DynamicSelectionList&& other)
+{
+    m_selections = std::move(other.m_selections);
+    if (m_buffer != other.m_buffer)
+    {
+        m_buffer->remove_change_listener(*this);
+        m_buffer = other.m_buffer;
+        m_buffer->add_change_listener(*this);
+    }
+    return *this;
+}
+
+void DynamicSelectionList::reset(SelectionList selections)
+{
+    for (auto& sel : selections)
+        assert(&sel.buffer() == m_buffer);
+     m_selections = std::move(selections);
+}
+
+void DynamicSelectionList::on_insert(const BufferIterator& begin, const BufferIterator& end)
+{
+    for (auto& sel : m_selections)
+    {
+        sel.first().on_insert(begin.coord(), end.coord());
+        sel.last().on_insert(begin.coord(), end.coord());
+        sel.check_invariant();
+    }
+}
+
+void DynamicSelectionList::on_erase(const BufferIterator& begin, const BufferIterator& end)
+{
+    for (auto& sel : m_selections)
+    {
+        sel.first().on_erase(begin.coord(), end.coord());
+        sel.last().on_erase(begin.coord(), end.coord());
+        sel.check_invariant();
+    }
+}
+
+}

src/dynamic_selection_list.hh

@@ -0,0 +1,67 @@
+#ifndef dynamic_selection_list_hh_INCLUDED
+#define dynamic_selection_list_hh_INCLUDED
+
+#include "selection.hh"
+
+namespace Kakoune
+{
+
+class DynamicSelectionList : public BufferChangeListener
+{
+public:
+    using iterator = SelectionList::iterator;
+    using const_iterator = SelectionList::const_iterator;
+
+    DynamicSelectionList(const Buffer& buffer, SelectionList selections = {});
+    ~DynamicSelectionList();
+
+    DynamicSelectionList(const DynamicSelectionList& other);
+    DynamicSelectionList& operator=(const DynamicSelectionList& other);
+    DynamicSelectionList(DynamicSelectionList&& other);
+    DynamicSelectionList& operator=(DynamicSelectionList&& other);
+
+    size_t size()  const { return m_selections.size(); }
+    bool   empty() const { return m_selections.empty(); }
+
+    void clear() { m_selections.clear(); }
+    iterator erase(iterator it) { return m_selections.erase(it); }
+
+    void push_back(Selection selection)
+    {
+        assert(&selection.buffer() == m_buffer);
+        m_selections.push_back(std::move(selection));
+    }
+
+    void reset(SelectionList selections);
+
+    iterator begin() { return m_selections.begin(); }
+    iterator end()   { return m_selections.end(); }
+    const_iterator begin() const { return m_selections.begin(); }
+    const_iterator end()   const { return m_selections.end(); }
+
+    Selection& front() { return m_selections.front(); }
+    Selection& back()  { return m_selections.back(); }
+    const Selection& front() const { return m_selections.front(); }
+    const Selection& back()  const { return m_selections.back(); }
+
+    Selection& operator[](size_t index) { return m_selections[index]; }
+    const Selection& operator[](size_t index) const { return m_selections[index]; }
+
+    operator const SelectionList&() const { return m_selections; }
+
+    const Buffer& buffer() const { return *m_buffer; }
+
+private:
+    void on_insert(const BufferIterator& begin,
+                   const BufferIterator& end) override;
+    void on_erase(const BufferIterator& begin,
+                  const BufferIterator& end) override;
+
+    const Buffer* m_buffer;
+    SelectionList m_selections;
+};
+
+};
+
+#endif // dynamic_selection_list_hh_INCLUDED
+

src/editor.cc

@@ -14,7 +14,8 @@ namespace Kakoune
 
 Editor::Editor(Buffer& buffer)
     : m_buffer(&buffer),
-      m_edition_level(0)
+      m_edition_level(0),
+      m_selections(buffer)
 {
     m_selections.push_back(Selection(buffer.begin(), buffer.begin()));
 }
@@ -118,7 +119,7 @@ std::vector<String> Editor::selections_content() const
     return contents;
 }
 
-static void merge_overlapping(SelectionList& selections)
+static void merge_overlapping(DynamicSelectionList& selections)
 {
     for (size_t i = 0; i < selections.size(); ++i)
     {
@@ -214,7 +215,7 @@ void Editor::select(SelectionList selections)
 {
     if (selections.empty())
         throw runtime_error("no selections");
-    m_selections = std::move(selections);
+    m_selections.reset(std::move(selections));
 }
 
 void Editor::select(const Selector& selector, SelectMode mode)
@@ -272,8 +273,8 @@ void Editor::multi_select(const MultiSelector& selector)
     if (new_selections.empty())
         throw nothing_selected();
 
-    merge_overlapping(new_selections);
-    m_selections = std::move(new_selections);
+    m_selections.reset(std::move(new_selections));
+    merge_overlapping(m_selections);
 }
 
 class LastModifiedRangeListener : public BufferChangeListener
@@ -316,7 +317,7 @@ bool Editor::undo()
     if (res)
     {
         m_selections.clear();
-        m_selections.emplace_back(listener.first(), listener.last());
+        m_selections.push_back({listener.first(), listener.last()});
     }
     return res;
 }
@@ -328,7 +329,7 @@ bool Editor::redo()
     if (res)
     {
         m_selections.clear();
-        m_selections.emplace_back(listener.first(), listener.last());
+        m_selections.push_back({listener.first(), listener.last()});
     }
     return res;
 }

src/editor.hh

@@ -2,7 +2,7 @@
 #define editor_hh_INCLUDED
 
 #include "buffer.hh"
-#include "selection.hh"
+#include "dynamic_selection_list.hh"
 #include "filter.hh"
 #include "idvaluemap.hh"
 #include "memoryview.hh"
@@ -95,7 +95,7 @@ private:
     virtual void on_incremental_insertion_end() {}
 
     safe_ptr<Buffer>         m_buffer;
-    SelectionList m_selections;
+    DynamicSelectionList     m_selections;
     FilterGroup              m_filters;
 };
 

src/main.cc

@@ -274,8 +274,8 @@ void do_indent(Context& context)
     String indent(' ', width);
 
     Editor& editor = context.editor();
-    SelectionList sels = editor.selections();
-    auto restore_sels = on_scope_end([&]{ editor.select(std::move(sels)); });
+    DynamicSelectionList sels{editor.buffer(), editor.selections()};
+    auto restore_sels = on_scope_end([&]{ editor.select((SelectionList)std::move(sels)); });
     editor.select(select_whole_lines);
     editor.multi_select(std::bind(select_all_matches, _1, "^[^\n]"));
     editor.insert(indent, InsertMode::Insert);
@@ -285,8 +285,8 @@ void do_deindent(Context& context)
 {
     int width = context.options()["indentwidth"].as_int();
     Editor& editor = context.editor();
-    SelectionList sels = editor.selections();
-    auto restore_sels = on_scope_end([&]{ editor.select(std::move(sels)); });
+    DynamicSelectionList sels{editor.buffer(), editor.selections()};
+    auto restore_sels = on_scope_end([&]{ editor.select((SelectionList)std::move(sels)); });
     editor.select(select_whole_lines);
     editor.multi_select(std::bind(select_all_matches, _1,
                                   "^\\h{1," + int_to_str(width) + "}"));
@@ -401,7 +401,7 @@ void jump(Context& context)
     auto jump = (direction == JumpDirection::Forward) ?
                  context.jump_forward() : context.jump_backward();
 
-    Buffer& buffer = const_cast<Buffer&>(jump.first().buffer());
+    Buffer& buffer = const_cast<Buffer&>(jump.front().buffer());
     BufferManager::instance().set_last_used_buffer(buffer);
     if (&buffer != &context.buffer())
     {

src/selection.cc

@@ -24,62 +24,10 @@ BufferIterator Range::end() const
     return utf8::next(std::max(m_first, m_last));
 }
 
-Selection::Selection(const BufferIterator& first, const BufferIterator& last,
-                     CaptureList captures)
-    : Range{first, last}, m_captures{std::move(captures)}
+void Range::check_invariant() const
 {
-    check_invariant();
-    register_with_buffer();
-}
-
-Selection::Selection(const Selection& other)
-    : Range(other), m_captures(other.m_captures)
-{
-   register_with_buffer();
-}
-
-Selection::Selection(Selection&& other)
-    : Range{other},
-      m_captures{std::move(other.m_captures)}
-{
-   register_with_buffer();
-}
-
-Selection::~Selection()
-{
-   unregister_with_buffer();
-}
-
-Selection& Selection::operator=(const Selection& other)
-{
-   const bool new_buffer = &first().buffer() != &other.first().buffer();
-   if (new_buffer)
-       unregister_with_buffer();
-
-   first() = other.first();
-   last()  = other.last();
-   m_captures = other.m_captures;
-
-   if (new_buffer)
-       register_with_buffer();
-
-   return *this;
-}
-
-Selection& Selection::operator=(Selection&& other)
-{
-   const bool new_buffer = &first().buffer() != &other.first().buffer();
-   if (new_buffer)
-       unregister_with_buffer();
-
-   first() = other.first();
-   last()  = other.last();
-   m_captures = std::move(other.m_captures);
-
-   if (new_buffer)
-       register_with_buffer();
-
-   return *this;
+    assert(utf8::is_character_start(first()));
+    assert(utf8::is_character_start(last()));
 }
 
 static void avoid_eol(BufferIterator& it)
@@ -95,34 +43,4 @@ void Selection::avoid_eol()
     Kakoune::avoid_eol(last());
 }
 
-void Selection::on_insert(const BufferIterator& begin, const BufferIterator& end)
-{
-    first().on_insert(begin.coord(), end.coord());
-    last().on_insert(begin.coord(), end.coord());
-    check_invariant();
-}
-
-void Selection::on_erase(const BufferIterator& begin, const BufferIterator& end)
-{
-    first().on_erase(begin.coord(), end.coord());
-    last().on_erase(begin.coord(), end.coord());
-    check_invariant();
-}
-
-void Selection::register_with_buffer()
-{
-    first().buffer().add_change_listener(*this);
-}
-
-void Selection::unregister_with_buffer()
-{
-    first().buffer().remove_change_listener(*this);
-}
-
-void Selection::check_invariant() const
-{
-    assert(utf8::is_character_start(first()));
-    assert(utf8::is_character_start(last()));
-}
-
 }

src/selection.hh

@@ -21,11 +21,17 @@ public:
     const BufferIterator& first() const { return m_first; }
     const BufferIterator& last() const { return m_last; }
 
+    bool operator== (const Range& other) const
+    {
+        return m_first == other.m_first and m_last == other.m_last;
+    }
+
     // returns min(first, last)
     BufferIterator begin() const;
     // returns max(first, last) + 1
     BufferIterator end() const;
 
+    void check_invariant() const;
 private:
     BufferIterator m_first;
     BufferIterator m_last;
@@ -40,33 +46,19 @@ inline bool overlaps(const Range& lhs, const Range& rhs)
 using CaptureList = std::vector<String>;
 
 // A selection is a Range, associated with a CaptureList
-// that updates itself when the buffer it points to gets modified.
-struct Selection : public Range, public BufferChangeListener
+struct Selection : public Range
 {
     Selection(const BufferIterator& first, const BufferIterator& last,
-              CaptureList captures = {});
-    Selection(Selection&& other);
-    Selection(const Selection& other);
-    ~Selection();
-
-    Selection& operator=(const Selection& other);
-    Selection& operator=(Selection&& other);
+              CaptureList captures = {})
+      : Range(first, last), m_captures(std::move(captures)) {}
 
     void avoid_eol();
 
     CaptureList& captures() { return m_captures; }
     const CaptureList& captures() const { return m_captures; }
 
+    const Buffer& buffer() const { return first().buffer(); }
 private:
-    void on_insert(const BufferIterator& begin,
-                   const BufferIterator& end) override;
-    void on_erase(const BufferIterator& begin,
-                  const BufferIterator& end) override;
-
-    void check_invariant() const;
-
-    void register_with_buffer();
-    void unregister_with_buffer();
 
     CaptureList m_captures;
 };
@@ -75,4 +67,3 @@ using SelectionList = std::vector<Selection>;
 }
 
 #endif // selection_hh_INCLUDED
-

src/window.cc

@@ -175,9 +175,9 @@ String Window::status_line() const
 
 void Window::on_incremental_insertion_end()
 {
-    SelectionList backup(selections());
+    DynamicSelectionList backup(buffer(), selections());
     hooks().run_hook("InsertEnd", "", Context(*this));
-    select(backup);
+    select((SelectionList)backup);
 }
 
 void Window::on_option_changed(const String& name, const Option& option)