Avoid the spurious newline insertion when replacing at end of buffer

Add a Buffer::replace method to handle the replacements properly
Fixes #633

src/buffer.cc

@@ -343,8 +343,9 @@ ByteCoord Buffer::do_insert(ByteCoord pos, StringView content)
         return pos;
 
     const bool at_end = is_end(pos);
+    const bool append_lines = at_end and (m_lines.empty() or byte_at(back_coord()) == '\n');
     if (at_end)
-        pos = line_count();
+        pos = append_lines ? line_count() : end_coord();
 
     const StringView prefix = at_end ?
         StringView{} : m_lines[pos.line].substr(0, pos.column);
@@ -369,7 +370,7 @@ ByteCoord Buffer::do_insert(ByteCoord pos, StringView content)
 
     auto line_it = m_lines.begin() + (int)pos.line;
     auto new_lines_it = new_lines.begin();
-    if (not at_end)
+    if (not append_lines)
         *line_it++ = std::move(*new_lines_it++);
 
     m_lines.insert(line_it,
@@ -473,6 +474,25 @@ BufferIterator Buffer::erase(BufferIterator begin, BufferIterator end)
     return {*this, do_erase(begin.coord(), end.coord())};
 }
 
+BufferIterator Buffer::replace(const BufferIterator& begin, const BufferIterator& end, StringView content)
+{
+    if (not (m_flags & Flags::NoUndo))
+        m_current_undo_group.emplace_back(Modification::Erase, begin.coord(),
+                                          intern(string(begin.coord(), end.coord())));
+    auto pos = do_erase(begin.coord(), end.coord());
+
+    StringDataPtr real_content;
+    if (is_end(pos) and content.back() != '\n')
+        real_content = intern(content + "\n");
+    else
+        real_content = intern(content);
+
+    auto coord = is_end(pos) ? ByteCoord{line_count()} : pos;
+    if (not (m_flags & Flags::NoUndo))
+        m_current_undo_group.emplace_back(Modification::Insert, coord, real_content);
+    return {*this, do_insert(pos, real_content->strview())};
+}
+
 bool Buffer::is_modified() const
 {
     size_t history_cursor_index = m_history_cursor - m_history.begin();

src/buffer.hh

@@ -123,6 +123,7 @@ public:
 
     BufferIterator insert(const BufferIterator& pos, StringView content);
     BufferIterator erase(BufferIterator begin, BufferIterator end);
+    BufferIterator replace(const BufferIterator& begin, const BufferIterator& end, StringView content);
 
     size_t         timestamp() const;
     timespec       fs_timestamp() const;

src/buffer_utils.hh

@@ -21,6 +21,13 @@ inline BufferIterator erase(Buffer& buffer, const Selection& range)
                         buffer.iterator_at(buffer.char_next(range.max())));
 }
 
+inline BufferIterator replace(Buffer& buffer, const Selection& range, StringView content)
+{
+    return buffer.replace(buffer.iterator_at(range.min()),
+                          buffer.iterator_at(buffer.char_next(range.max())),
+                          content);
+}
+
 inline CharCount char_length(const Buffer& buffer, const Selection& range)
 {
     return utf8::distance(buffer.iterator_at(range.min()),

src/selection.cc

@@ -439,7 +439,7 @@ BufferIterator prepare_insert(Buffer& buffer, const Selection& sel, InsertMode m
     case InsertMode::InsertCursor:
         return buffer.iterator_at(sel.cursor());
     case InsertMode::Replace:
-        return erase(buffer, sel);
+        return {}; // replace is handled specially, by calling Buffer::replace
     case InsertMode::Append:
     {
         // special case for end of lines, append to current line instead
@@ -482,21 +482,24 @@ void SelectionList::insert(ConstArrayView<String> strings, InsertMode mode,
         changes_tracker.update(*m_buffer, m_timestamp);
 
         const String& str = strings[std::min(index, strings.size()-1)];
-        if (str.empty())
-        {
-            if (mode == InsertMode::Replace)
-                sel.anchor() = sel.cursor() = m_buffer->clamp(pos.coord());
-            continue;
-        }
 
-        pos = m_buffer->insert(pos, str);
+        if (mode == InsertMode::Replace)
+            pos = replace(*m_buffer, sel, str);
+        else
+            pos = m_buffer->insert(pos, str);
 
         auto& change = m_buffer->changes_since(m_timestamp).back();
-        changes_tracker.update(change);
+        changes_tracker.update(*m_buffer, m_timestamp);
         m_timestamp = m_buffer->timestamp();
 
         if (select_inserted or mode == InsertMode::Replace)
         {
+            if (str.empty())
+            {
+                sel.anchor() = sel.cursor() = m_buffer->clamp(pos.coord());
+                continue;
+            }
+
             // we want min and max from *before* we do any change
             auto& min = sel.min();
             auto& max = sel.max();
@@ -505,6 +508,9 @@ void SelectionList::insert(ConstArrayView<String> strings, InsertMode mode,
         }
         else
         {
+            if (str.empty())
+                continue;
+
             sel.anchor() = m_buffer->clamp(update_insert(sel.anchor(), change.begin, change.end));
             sel.cursor() = m_buffer->clamp(update_insert(sel.cursor(), change.begin, change.end));
         }

test/regression/633-spurious-new-line-inserted-when-replacing-at-end/cmd

@@ -0,0 +1 @@
+x~

test/regression/633-spurious-new-line-inserted-when-replacing-at-end/in

@@ -0,0 +1 @@
+blah

test/regression/633-spurious-new-line-inserted-when-replacing-at-end/out

@@ -0,0 +1 @@
+BLAH