Selection: do not use [begin, end) semantics but [first, last]

selections are now defined with inclusive iterators, which means that
Selection(cursor, cursor) is a valid selection of the charateter pointed
by cursor.

On the user interface side, that means that the cursor is now part of
the selection, selectors were adapted to this behavior (and word
selectors are now much more intuitive)

src/regex_selector.cc

@@ -16,9 +16,9 @@ Selection RegexSelector::operator()(const BufferIterator& cursor) const
         boost::match_results<BufferIterator> matches;
 
         if (boost::regex_search(cursor, cursor.buffer().end(), matches, m_regex, boost::match_nosubs))
-            return Selection(matches.begin()->first, matches.begin()->second);
+            return Selection(matches.begin()->first, matches.begin()->second-1);
         else if (boost::regex_search(cursor.buffer().begin(), cursor, matches, m_regex, boost::match_nosubs))
-            return Selection(matches.begin()->first, matches.begin()->second);
+            return Selection(matches.begin()->first, matches.begin()->second-1);
     }
     catch (boost::regex_error& err)
     {

src/selectors.cc

@@ -33,6 +33,22 @@ static bool is_punctuation(char c)
     return not (is_word(c) or is_blank(c));
 }
 
+enum class CharCategories
+{
+    Blank,
+    Word,
+    Punctuation,
+};
+
+static CharCategories categorize(char c)
+{
+    if (is_word(c))
+        return CharCategories::Word;
+    if (is_blank(c))
+        return CharCategories::Blank;
+    return CharCategories::Punctuation;
+}
+
 template<typename T>
 void skip_while(BufferIterator& it, T condition)
 {
@@ -50,55 +66,72 @@ void skip_while_reverse(BufferIterator& it, T condition)
 
 Selection select_to_next_word(const BufferIterator& cursor)
 {
-    BufferIterator end = cursor;
+    BufferIterator begin = cursor;
+    BufferIterator end = cursor+1;
 
-    if (is_word(*end))
-        skip_while(end, is_word);
-    else if (is_punctuation(*end))
+    if (categorize(*begin) != categorize(*end))
+    {
+        ++begin;
+        ++end;
+    }
+
+    if (is_punctuation(*begin))
         skip_while(end, is_punctuation);
 
+    if (is_word(*begin))
+        skip_while(end, is_word);
+
     skip_while(end, is_blank);
 
-    return Selection(cursor, end);
+    return Selection(begin, end-1);
 }
 
 Selection select_to_next_word_end(const BufferIterator& cursor)
 {
-    BufferIterator end = cursor;
+    BufferIterator begin = cursor;
+    BufferIterator end = cursor+1;
+
+    if (categorize(*begin) != categorize(*end))
+        ++begin;
 
     skip_while(end, is_blank);
 
-    if (is_word(*end))
-        skip_while(end, is_word);
-    else if (is_punctuation(*end))
+    if (is_punctuation(*end))
         skip_while(end, is_punctuation);
+    else if (is_word(*end))
+        skip_while(end, is_word);
 
-    return Selection(cursor, end);
+    return Selection(begin, end-1);
 }
 
 Selection select_to_previous_word(const BufferIterator& cursor)
 {
-    BufferIterator end = cursor;
+    BufferIterator begin = cursor;
+    BufferIterator end = cursor-1;
+
+    if (categorize(*begin) != categorize(*end))
+        --begin;
 
     skip_while_reverse(end, is_blank);
-    if (is_word(*end))
-        skip_while_reverse(end, is_word);
-    else if (is_punctuation(*end))
-        skip_while_reverse(end, is_punctuation);
 
-    return Selection(cursor, end);
+    if (is_punctuation(*end))
+        skip_while_reverse(end, is_punctuation);
+    else if (is_word(*end))
+        skip_while_reverse(end, is_word);
+
+    return Selection(begin, end+1);
 }
 
 Selection select_line(const BufferIterator& cursor)
 {
-    BufferIterator begin = cursor;
-    while (not begin.is_begin() and *(begin -1) != '\n')
-        --begin;
+    BufferIterator first = cursor;
+    while (not first.is_begin() and *(first - 1) != '\n')
+        --first;
 
-    BufferIterator end = cursor;
-    while (not end.is_end() and *end != '\n')
-        ++end;
-    return Selection(begin, end + 1);
+    BufferIterator last = cursor;
+    while (not (last + 1).is_end() and *last != '\n')
+        ++last;
+    return Selection(first, last);
 }
 
 Selection move_select(Window& window, const BufferIterator& cursor, const WindowCoord& offset)
@@ -136,7 +169,7 @@ Selection select_matching(const BufferIterator& cursor)
             if (*it == opening)
                 ++level;
             else if (*it == closing and --level == 0)
-                return Selection(begin, it+1);
+                return Selection(begin, it);
 
             ++it;
         }
@@ -151,7 +184,7 @@ Selection select_matching(const BufferIterator& cursor)
             if (*it == closing)
                 ++level;
             else if (*it == opening and --level == 0)
-                return Selection(begin, it-1);
+                return Selection(begin, it);
             --it;
         }
     }
@@ -163,7 +196,7 @@ Selection select_to(const BufferIterator& cursor, char c)
     BufferIterator end = cursor + 1;
     skip_while(end, [c](char cur) { return not is_eol(cur) and cur != c; });
     if (not is_eol(*end))
-        return Selection(cursor, end);
+        return Selection(cursor, end-1);
     return Selection(cursor, cursor);
 }
 

src/window.cc

@@ -7,6 +7,22 @@
 namespace Kakoune
 {
 
+BufferIterator Selection::begin() const
+{
+    return std::min(m_first, m_last);
+}
+
+BufferIterator Selection::end() const
+{
+    return std::max(m_first, m_last) + 1;
+}
+
+void Selection::offset(int offset)
+{
+    m_first += offset;
+    m_last += offset;
+}
+
 Window::Window(Buffer& buffer)
     : m_buffer(buffer),
       m_position(0, 0),
@@ -27,12 +43,11 @@ void Window::erase()
 
     for (auto& sel : m_selections)
     {
-        sel.canonicalize();
         m_buffer.erase(sel.begin(), sel.end());
         sel = Selection(sel.begin(), sel.begin());
     }
     if (not m_selections.empty())
-        m_cursor = line_and_column_at(m_selections.back().end());
+        m_cursor = line_and_column_at(m_selections.back().last());
     m_buffer.end_undo_group();
 }
 
@@ -133,10 +148,10 @@ void Window::select(bool append, const Selector& selector)
     {
         for (auto& sel : m_selections)
         {
-            sel = Selection(sel.begin(), selector(sel.end()).end());
+            sel = Selection(sel.first(), selector(sel.last()).last());
         }
     }
-    m_cursor = line_and_column_at(m_selections.back().end());
+    m_cursor = line_and_column_at(m_selections.back().last());
     scroll_to_keep_cursor_visible_ifn();
 }
 
@@ -160,9 +175,6 @@ void Window::update_display_buffer()
 
     SelectionList sorted_selections = m_selections;
 
-    for (auto& sel : sorted_selections)
-        sel.canonicalize();
-
     std::sort(sorted_selections.begin(), sorted_selections.end(),
               [](const Selection& lhs, const Selection& rhs) { return lhs.begin() < rhs.begin(); });
 

src/window.hh

@@ -19,27 +19,20 @@ struct WindowCoord : LineAndColumn<WindowCoord>
 
 struct Selection
 {
-    Selection(const BufferIterator& begin, const BufferIterator& end)
-        : m_begin(begin), m_end(end) {}
+    Selection(const BufferIterator& first, const BufferIterator& last)
+        : m_first(first), m_last(last) {}
 
-    const BufferIterator& begin() const { return m_begin; }
-    const BufferIterator& end() const   { return m_end; }
+    BufferIterator begin() const;
+    BufferIterator end() const;
 
-    void canonicalize()
-    {
-        if (m_end < m_begin)
-            std::swap(++m_begin, ++m_end);
-    }
+    const BufferIterator& first() const { return m_first; }
+    const BufferIterator& last()  const { return m_last; }
 
-    void offset(int offset)
-    {
-        m_begin += offset;
-        m_end += offset;
-    }
+    void offset(int offset);
 
 private:
-    BufferIterator m_begin;
-    BufferIterator m_end;
+    BufferIterator m_first;
+    BufferIterator m_last;
 };
 
 typedef std::vector<Selection> SelectionList;