From b68490ef11734c404f8a18f1babd8004aca2de06 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Wed, 18 Dec 2019 11:36:17 +1100 Subject: [PATCH] Cleanup replaced range selection logic Do not access Buffer::m_changes to find the inserted range, return it directly from Buffer::insert and Buffer::replace. This fixes a wrong behaviour where replacing at eof would lose the selected end of line (as the implementation does not actually replace that end of line) --- src/buffer.cc | 27 ++++++------- src/buffer.hh | 8 ++-- src/buffer_utils.hh | 2 +- src/normal.cc | 2 +- src/selection.cc | 40 ++++++------------- .../0-rotate-at-eof-mutates-selection/cmd | 1 + .../0-rotate-at-eof-mutates-selection/in | 3 ++ .../0-rotate-at-eof-mutates-selection/out | 3 ++ 8 files changed, 40 insertions(+), 46 deletions(-) create mode 100644 test/regression/0-rotate-at-eof-mutates-selection/cmd create mode 100644 test/regression/0-rotate-at-eof-mutates-selection/in create mode 100644 test/regression/0-rotate-at-eof-mutates-selection/out diff --git a/src/buffer.cc b/src/buffer.cc index 4a697dd8..0d738dad 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -444,12 +444,12 @@ void Buffer::check_invariant() const #endif } -BufferCoord Buffer::do_insert(BufferCoord pos, StringView content) +BufferRange Buffer::do_insert(BufferCoord pos, StringView content) { kak_assert(is_valid(pos)); if (content.empty()) - return pos; + return {pos, pos}; const bool at_end = is_end(pos); const bool append_lines = at_end and (m_lines.empty() or byte_at(back_coord()) == '\n'); @@ -489,7 +489,7 @@ BufferCoord Buffer::do_insert(BufferCoord pos, StringView content) : BufferCoord{ last_line, m_lines[last_line].length() - suffix.length() }; m_changes.push_back({ Change::Insert, pos, end }); - return end; + return {pos, end}; } BufferCoord Buffer::do_erase(BufferCoord begin, BufferCoord end) @@ -543,13 +543,13 @@ void Buffer::apply_modification(const Modification& modification) } } -BufferCoord Buffer::insert(BufferCoord pos, StringView content) +BufferRange Buffer::insert(BufferCoord pos, StringView content) { throw_if_read_only(); kak_assert(is_valid(pos)); if (content.empty()) - return pos; + return {pos, pos}; StringDataPtr real_content; if (is_end(pos) and content.back() != '\n') @@ -584,21 +584,20 @@ BufferCoord Buffer::erase(BufferCoord begin, BufferCoord end) return do_erase(begin, end); } -BufferCoord Buffer::replace(BufferCoord begin, BufferCoord end, StringView content) +BufferRange Buffer::replace(BufferCoord begin, BufferCoord end, StringView content) { throw_if_read_only(); + if (std::equal(iterator_at(begin), iterator_at(end), content.begin(), content.end())) + return {begin, end}; + if (is_end(end) and not content.empty() and content.back() == '\n') { - end = back_coord(); - content = content.substr(0, content.length() - 1); + auto pos = insert(erase(begin, back_coord()), + content.substr(0, content.length() - 1)).begin; + return {pos, end_coord()}; } - - if (std::equal(iterator_at(begin), iterator_at(end), content.begin(), content.end())) - return begin; - - auto pos = erase(begin, end); - return insert(pos, content); + return insert(erase(begin, end), content); } bool Buffer::is_modified() const diff --git a/src/buffer.hh b/src/buffer.hh index 4f27e449..a2517956 100644 --- a/src/buffer.hh +++ b/src/buffer.hh @@ -7,6 +7,7 @@ #include "enum.hh" #include "file.hh" #include "optional.hh" +#include "range.hh" #include "safe_ptr.hh" #include "scope.hh" #include "shared_string.hh" @@ -103,6 +104,7 @@ private: }; using BufferLines = Vector; +using BufferRange = Range; // A Buffer is a in-memory representation of a file // @@ -139,9 +141,9 @@ public: bool set_name(String name); void update_display_name(); - BufferCoord insert(BufferCoord pos, StringView content); + BufferRange insert(BufferCoord pos, StringView content); BufferCoord erase(BufferCoord begin, BufferCoord end); - BufferCoord replace(BufferCoord begin, BufferCoord end, StringView content); + BufferRange replace(BufferCoord begin, BufferCoord end, StringView content); size_t timestamp() const; void set_fs_status(FsStatus); @@ -231,7 +233,7 @@ public: private: void on_option_changed(const Option& option) override; - BufferCoord do_insert(BufferCoord pos, StringView content); + BufferRange do_insert(BufferCoord pos, StringView content); BufferCoord do_erase(BufferCoord begin, BufferCoord end); struct Modification; diff --git a/src/buffer_utils.hh b/src/buffer_utils.hh index 586cbb1d..a3e43068 100644 --- a/src/buffer_utils.hh +++ b/src/buffer_utils.hh @@ -20,7 +20,7 @@ inline BufferCoord erase(Buffer& buffer, const Selection& range) return buffer.erase(range.min(), buffer.char_next(range.max())); } -inline BufferCoord replace(Buffer& buffer, const Selection& range, StringView content) +inline BufferRange replace(Buffer& buffer, const Selection& range, StringView content) { return buffer.replace(range.min(), buffer.char_next(range.max()), content); } diff --git a/src/normal.cc b/src/normal.cc index bd4f5ee2..f9b1184f 100644 --- a/src/normal.cc +++ b/src/normal.cc @@ -524,7 +524,7 @@ BufferCoord apply_diff(Buffer& buffer, BufferCoord pos, StringView before, Strin break; case DiffOp::Add: pos = buffer.insert(pos, {lines_after[posB].begin(), - lines_after[posB + len - 1].end()}); + lines_after[posB + len - 1].end()}).end; break; case DiffOp::Remove: pos = buffer.erase(pos, buffer.advance(pos, byte_count(lines_before, posA, len))); diff --git a/src/selection.cc b/src/selection.cc index 0a0983a5..781f7c4f 100644 --- a/src/selection.cc +++ b/src/selection.cc @@ -407,38 +407,24 @@ void SelectionList::insert(ConstArrayView strings, InsertMode mode, sel.min() : changes_tracker.get_new_coord(insert_pos[index]); if (mode == InsertMode::Replace) - replace(*m_buffer, sel, str); + { + auto range = replace(*m_buffer, sel, str); + // we want min and max from *before* we do any change + auto& min = sel.min(); + auto& max = sel.max(); + min = range.begin; + max = m_buffer->char_prev(range.end); + } else - m_buffer->insert(pos, str); + { + auto range = m_buffer->insert(pos, str); + sel.anchor() = m_buffer->clamp(update_insert(sel.anchor(), range.begin, range.end)); + sel.cursor() = m_buffer->clamp(update_insert(sel.cursor(), range.begin, range.end)); + } - size_t old_timestamp = m_timestamp; changes_tracker.update(*m_buffer, m_timestamp); - if (out_insert_pos) out_insert_pos->push_back(pos); - - if (mode == InsertMode::Replace) - { - auto changes = m_buffer->changes_since(old_timestamp); - if (changes.size() == 1) // Nothing got inserted, either str was empty, or just \n at end of buffer - sel.anchor() = sel.cursor() = m_buffer->clamp(pos); - else if (changes.size() == 2) - { - // we want min and max from *before* we do any change - auto& min = sel.min(); - auto& max = sel.max(); - min = changes.back().begin; - max = m_buffer->char_prev(changes.back().end); - } - else - kak_assert(changes.empty()); - } - else if (not str.empty()) - { - auto& change = m_buffer->changes_since(0).back(); - 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)); - } } // We might just have been deleting text if strings were empty, diff --git a/test/regression/0-rotate-at-eof-mutates-selection/cmd b/test/regression/0-rotate-at-eof-mutates-selection/cmd new file mode 100644 index 00000000..92824684 --- /dev/null +++ b/test/regression/0-rotate-at-eof-mutates-selection/cmd @@ -0,0 +1 @@ +% diff --git a/test/regression/0-rotate-at-eof-mutates-selection/in b/test/regression/0-rotate-at-eof-mutates-selection/in new file mode 100644 index 00000000..86e041da --- /dev/null +++ b/test/regression/0-rotate-at-eof-mutates-selection/in @@ -0,0 +1,3 @@ +foo +bar +baz diff --git a/test/regression/0-rotate-at-eof-mutates-selection/out b/test/regression/0-rotate-at-eof-mutates-selection/out new file mode 100644 index 00000000..aee7988d --- /dev/null +++ b/test/regression/0-rotate-at-eof-mutates-selection/out @@ -0,0 +1,3 @@ +bar +baz +foo