From f6f0c11be4fc9cb230b3e75213f6aa8f7584cd14 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sun, 7 Dec 2025 00:30:11 -0800 Subject: [PATCH] Add PieceTable-based buffer tests and improvements for file I/O and editing. - Introduced comprehensive tests: - `test_buffer_open_nonexistent_save.cc`: Save after opening a non-existent file. - `test_buffer_save.cc`: Save buffer contents to disk. - `test_buffer_save_existing.cc`: Save after opening existing files. - Implemented `PieceTable::WriteToStream()` to directly stream content without full materialization. - Updated `Buffer::Save` and `Buffer::SaveAs` to use efficient streaming via `PieceTable`. - Enhanced editing commands (`Insert`, `Delete`, `Replace`, etc.) to use PieceTable APIs, ensuring proper undo and save functionality. --- .idea/codeStyles/codeStyleConfig.xml | 1 + Buffer.cc | 65 +- CMakeLists.txt | 41 ++ Command.cc | 885 +++++++++++++-------------- Editor.cc | 14 +- PieceTable.cc | 15 + PieceTable.h | 4 + test_buffer_open_nonexistent_save.cc | 50 ++ test_buffer_save.cc | 57 ++ test_buffer_save_existing.cc | 48 ++ 10 files changed, 682 insertions(+), 498 deletions(-) create mode 100644 test_buffer_open_nonexistent_save.cc create mode 100644 test_buffer_save.cc create mode 100644 test_buffer_save_existing.cc diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml index b8e8d6d..c308372 100644 --- a/.idea/codeStyles/codeStyleConfig.xml +++ b/.idea/codeStyles/codeStyleConfig.xml @@ -1,5 +1,6 @@ + \ No newline at end of file diff --git a/Buffer.cc b/Buffer.cc index 6ce1e5e..4b6fa36 100644 --- a/Buffer.cc +++ b/Buffer.cc @@ -292,28 +292,29 @@ Buffer::OpenFromFile(const std::string &path, std::string &err) bool Buffer::Save(std::string &err) const { - if (!is_file_backed_ || filename_.empty()) { - err = "Buffer is not file-backed; use SaveAs()"; - return false; - } - std::ofstream out(filename_, std::ios::out | std::ios::binary | std::ios::trunc); - if (!out) { - err = "Failed to open for write: " + filename_ + ". Error: " + std::string(std::strerror(errno)); - return false; - } - // Write the entire buffer in a single block to minimize I/O calls. - const char *data = content_.Data(); - const auto size = static_cast(content_.Size()); - if (data != nullptr && size > 0) { - out.write(data, size); - } - if (!out.good()) { - err = "Write error: " + filename_ + ". Error: " + std::string(std::strerror(errno)); - return false; - } - // Note: const method cannot change dirty_. Intentionally const to allow UI code - // to decide when to flip dirty flag after successful save. - return true; + if (!is_file_backed_ || filename_.empty()) { + err = "Buffer is not file-backed; use SaveAs()"; + return false; + } + std::ofstream out(filename_, std::ios::out | std::ios::binary | std::ios::trunc); + if (!out) { + err = "Failed to open for write: " + filename_ + ". Error: " + std::string(std::strerror(errno)); + return false; + } + // Stream the content directly from the piece table to avoid relying on + // full materialization, which may yield an empty pointer when size > 0. + if (content_.Size() > 0) { + content_.WriteToStream(out); + } + // Ensure data hits the OS buffers + out.flush(); + if (!out.good()) { + err = "Write error: " + filename_ + ". Error: " + std::string(std::strerror(errno)); + return false; + } + // Note: const method cannot change dirty_. Intentionally const to allow UI code + // to decide when to flip dirty flag after successful save. + return true; } @@ -345,16 +346,16 @@ Buffer::SaveAs(const std::string &path, std::string &err) err = "Failed to open for write: " + out_path + ". Error: " + std::string(std::strerror(errno)); return false; } - // Write whole content in a single I/O operation - const char *data = content_.Data(); - const auto size = static_cast(content_.Size()); - if (data != nullptr && size > 0) { - out.write(data, size); - } - if (!out.good()) { - err = "Write error: " + out_path + ". Error: " + std::string(std::strerror(errno)); - return false; - } + // Stream content without forcing full materialization + if (content_.Size() > 0) { + content_.WriteToStream(out); + } + // Ensure data hits the OS buffers + out.flush(); + if (!out.good()) { + err = "Write error: " + out_path + ". Error: " + std::string(std::strerror(errno)); + return false; + } filename_ = out_path; is_file_backed_ = true; diff --git a/CMakeLists.txt b/CMakeLists.txt index 91b3ed1..afc85ca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -313,6 +313,47 @@ if (BUILD_TESTS) target_link_libraries(test_undo ${TREESITTER_LIBRARY}) endif () endif () + + # test_buffer_save executable to verify Buffer::Save writes contents to disk + # Keep this test minimal to avoid pulling the entire app; only compile what's needed + add_executable(test_buffer_save + test_buffer_save.cc + PieceTable.cc + Buffer.cc + ${SYNTAX_SOURCES} + UndoNode.cc + UndoTree.cc + UndoSystem.cc + ) + # test_buffer_save_existing: verifies Save() after OpenFromFile on existing path + add_executable(test_buffer_save_existing + test_buffer_save_existing.cc + PieceTable.cc + Buffer.cc + ${SYNTAX_SOURCES} + UndoNode.cc + UndoTree.cc + UndoSystem.cc + ) + # test for opening a non-existent path then saving + add_executable(test_buffer_open_nonexistent_save + test_buffer_open_nonexistent_save.cc + PieceTable.cc + Buffer.cc + ${SYNTAX_SOURCES} + UndoNode.cc + UndoTree.cc + UndoSystem.cc + ) + # No ncurses needed for this unit + if (KTE_ENABLE_TREESITTER) + if (TREESITTER_INCLUDE_DIR) + target_include_directories(test_buffer_save PRIVATE ${TREESITTER_INCLUDE_DIR}) + endif () + if (TREESITTER_LIBRARY) + target_link_libraries(test_buffer_save ${TREESITTER_LIBRARY}) + endif () + endif () endif () if (${BUILD_GUI}) diff --git a/Command.cc b/Command.cc index 2327431..c5b2514 100644 --- a/Command.cc +++ b/Command.cc @@ -1967,29 +1967,34 @@ cmd_insert_text(CommandContext &ctx) ctx.editor.SetStatus("InsertText arg must not contain newlines"); return false; } - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - if (y >= rows.size()) { - rows.resize(y + 1); - } - int repeat = ctx.count > 0 ? ctx.count : 1; - for (int i = 0; i < repeat; ++i) { - rows[y].insert(x, ctx.arg); - x += ctx.arg.size(); - } - buf->SetDirty(true); - // Record undo after buffer modification but before cursor update - if (auto *u = buf->Undo()) { - u->Begin(UndoType::Insert); - for (int i = 0; i < repeat; ++i) { - u->Append(std::string_view(ctx.arg)); - } - } - buf->SetCursor(x, y); - ensure_cursor_visible(ctx.editor, *buf); - return true; + ensure_at_least_one_line(*buf); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + std::size_t ins_y = y; + std::size_t ins_x = x; // remember insertion start for undo positioning + int repeat = ctx.count > 0 ? ctx.count : 1; + + // Apply edits to the underlying PieceTable through Buffer::insert_text, + // not directly to the legacy rows_ cache. This ensures Save() persists text. + for (int i = 0; i < repeat; ++i) { + buf->insert_text(static_cast(y), static_cast(x), std::string_view(ctx.arg)); + x += ctx.arg.size(); + } + buf->SetDirty(true); + // Record undo for this contiguous insert at the original insertion point + if (auto *u = buf->Undo()) { + // Position cursor at insertion start for the undo record + buf->SetCursor(ins_x, ins_y); + u->Begin(UndoType::Insert); + for (int i = 0; i < repeat; ++i) { + u->Append(std::string_view(ctx.arg)); + } + // Finalize this contiguous insert as a single undoable action + u->commit(); + } + buf->SetCursor(x, y); + ensure_cursor_visible(ctx.editor, *buf); + return true; } @@ -2320,53 +2325,57 @@ cmd_newline(CommandContext &ctx) // Save original cursor to restore after operations std::size_t orig_x = buf->Curx(); std::size_t orig_y = buf->Cury(); - auto &rows = buf->Rows(); - std::size_t total = 0; - UndoSystem *u = buf->Undo(); - if (u) - u->commit(); // end any pending batch - for (std::size_t y = 0; y < rows.size(); ++y) { - std::size_t pos = 0; - while (!find.empty()) { - pos = rows[y].find(find, pos); - if (pos == std::string::npos) - break; - // Perform delete of matched segment - rows[y].erase(pos, find.size()); - if (u) { - buf->SetCursor(pos, y); - u->Begin(UndoType::Delete); - u->Append(std::string_view(find)); - } - // Insert replacement - if (!with.empty()) { - rows[y].insert(pos, with); - if (u) { - buf->SetCursor(pos, y); - u->Begin(UndoType::Insert); - u->Append(std::string_view(with)); - } - pos += with.size(); - } - ++total; - if (with.empty()) { - // Avoid infinite loop when replacing with empty - // pos remains the same; move forward by 1 to continue search - if (pos < rows[y].size()) - ++pos; - else - break; - } - } - } - buf->SetDirty(true); - // Restore original cursor - if (orig_y < rows.size()) - buf->SetCursor(orig_x, orig_y); - ensure_cursor_visible(ctx.editor, *buf); - char msg[128]; - std::snprintf(msg, sizeof(msg), "Replaced %zu occurrence%s", total, (total == 1 ? "" : "s")); - ctx.editor.SetStatus(msg); + std::size_t total = 0; + UndoSystem *u = buf->Undo(); + if (u) + u->commit(); // end any pending batch + for (std::size_t y = 0; y < buf->Rows().size(); ++y) { + std::size_t pos = 0; + while (true) { + const auto &rows_view = buf->Rows(); + if (y >= rows_view.size()) + break; + std::string line = static_cast(rows_view[y]); + if (find.empty()) + break; + std::size_t p = line.find(find, pos); + if (p == std::string::npos) + break; + // Delete matched segment from the piece table + buf->delete_text(static_cast(y), static_cast(p), find.size()); + if (u) { + buf->SetCursor(p, y); + u->Begin(UndoType::Delete); + u->Append(std::string_view(find)); + } + // Insert replacement if provided + if (!with.empty()) { + buf->insert_text(static_cast(y), static_cast(p), std::string_view(with)); + if (u) { + buf->SetCursor(p, y); + u->Begin(UndoType::Insert); + u->Append(std::string_view(with)); + } + pos = p + with.size(); + } else { + // When replacing with empty, continue after the deletion point to avoid re-matching + pos = p; + if (pos < static_cast(buf->Rows()[y].size())) + ++pos; + else + break; + } + ++total; + } + } + buf->SetDirty(true); + // Restore original cursor + if (orig_y < buf->Rows().size()) + buf->SetCursor(orig_x, orig_y); + ensure_cursor_visible(ctx.editor, *buf); + char msg[128]; + std::snprintf(msg, sizeof(msg), "Replaced %zu occurrence%s", total, (total == 1 ? "" : "s")); + ctx.editor.SetStatus(msg); // Clear search-highlighting state after replace completes ctx.editor.SetSearchActive(false); ctx.editor.SetSearchQuery(""); @@ -2374,7 +2383,7 @@ cmd_newline(CommandContext &ctx) ctx.editor.ClearSearchOrigin(); ctx.editor.SetSearchIndex(-1); return true; - } else if (kind == Editor::PromptKind::OpenFile) { + } else if (kind == Editor::PromptKind::OpenFile) { std::string err; // Expand "~" to the user's home directory auto expand_user_path = [](const std::string &in) -> std::string { @@ -2392,12 +2401,14 @@ cmd_newline(CommandContext &ctx) value = expand_user_path(value); if (value.empty()) { ctx.editor.SetStatus("Open canceled (empty)"); - } else if (!ctx.editor.OpenFile(value, err)) { - ctx.editor.SetStatus(err.empty() ? std::string("Failed to open ") + value : err); - } else { - ctx.editor.SetStatus(std::string("Opened ") + value); - } - } else if (kind == Editor::PromptKind::BufferSwitch) { + } else if (!ctx.editor.OpenFile(value, err)) { + ctx.editor.SetStatus(err.empty() ? std::string("Failed to open ") + value : err); + } else { + ctx.editor.SetStatus(std::string("Opened ") + value); + // Close the prompt so subsequent typing edits the buffer, not the prompt + ctx.editor.CancelPrompt(); + } + } else if (kind == Editor::PromptKind::BufferSwitch) { // Resolve to a buffer index by exact match against path or basename; // if multiple partial matches, prefer exact; if none, keep status. const auto &bs = ctx.editor.Buffers(); @@ -2458,16 +2469,18 @@ cmd_newline(CommandContext &ctx) std::string("Overwrite existing file '") + value + "'? (y/N)"); } else { std::string err; - if (!buf->SaveAs(value, err)) { - ctx.editor.SetStatus(err); - } else { - buf->SetDirty(false); - ctx.editor.SetStatus("Saved as " + value); - if (auto *u = buf->Undo()) - u->mark_saved(); - // If a close-after-save was requested (from closing a dirty, unnamed buffer), - // close the buffer now. - if (ctx.editor.CloseAfterSave()) { + if (!buf->SaveAs(value, err)) { + ctx.editor.SetStatus(err); + } else { + buf->SetDirty(false); + ctx.editor.SetStatus("Saved as " + value); + if (auto *u = buf->Undo()) + u->mark_saved(); + // Close the prompt on successful save-as + ctx.editor.CancelPrompt(); + // If a close-after-save was requested (from closing a dirty, unnamed buffer), + // close the buffer now. + if (ctx.editor.CloseAfterSave()) { ctx.editor.SetCloseAfterSave(false); std::size_t idx_close = ctx.editor.CurrentBufferIndex(); std::string name_close = buffer_display_name(*buf); @@ -2495,47 +2508,51 @@ cmd_newline(CommandContext &ctx) // Confirmation for potentially destructive operations (e.g., overwrite on save-as) Buffer *buf = ctx.editor.CurrentBuffer(); const std::string target = ctx.editor.PendingOverwritePath(); - if (!target.empty() && buf) { - bool yes = false; - if (!value.empty()) { - char c = value[0]; - yes = (c == 'y' || c == 'Y'); - } - if (yes) { - std::string err; - if (!buf->SaveAs(target, err)) { - ctx.editor.SetStatus(err); - } else { - buf->SetDirty(false); - ctx.editor.SetStatus("Saved as " + target); - if (auto *u = buf->Undo()) - u->mark_saved(); - // If this overwrite confirm was part of a close-after-save flow, close now. - if (ctx.editor.CloseAfterSave()) { - ctx.editor.SetCloseAfterSave(false); - std::size_t idx_close = ctx.editor.CurrentBufferIndex(); - std::string name_close = buffer_display_name(*buf); - if (buf->Undo()) - buf->Undo()->discard_pending(); - ctx.editor.CloseBuffer(idx_close); - if (ctx.editor.BufferCount() == 0) { - Buffer empty; - ctx.editor.AddBuffer(std::move(empty)); - ctx.editor.SwitchTo(0); - } - const Buffer *cur = ctx.editor.CurrentBuffer(); - ctx.editor.SetStatus( - std::string("Closed: ") + name_close + std::string( - " Now: ") - + (cur ? buffer_display_name(*cur) : std::string(""))); - } - } - } else { - ctx.editor.SetStatus("Save canceled"); - } - ctx.editor.ClearPendingOverwritePath(); - // Regardless of answer, end any close-after-save pending state for safety. - ctx.editor.SetCloseAfterSave(false); + if (!target.empty() && buf) { + bool yes = false; + if (!value.empty()) { + char c = value[0]; + yes = (c == 'y' || c == 'Y'); + } + if (yes) { + std::string err; + if (!buf->SaveAs(target, err)) { + ctx.editor.SetStatus(err); + } else { + buf->SetDirty(false); + ctx.editor.SetStatus("Saved as " + target); + if (auto *u = buf->Undo()) + u->mark_saved(); + // If this overwrite confirm was part of a close-after-save flow, close now. + if (ctx.editor.CloseAfterSave()) { + ctx.editor.SetCloseAfterSave(false); + std::size_t idx_close = ctx.editor.CurrentBufferIndex(); + std::string name_close = buffer_display_name(*buf); + if (buf->Undo()) + buf->Undo()->discard_pending(); + ctx.editor.CloseBuffer(idx_close); + if (ctx.editor.BufferCount() == 0) { + Buffer empty; + ctx.editor.AddBuffer(std::move(empty)); + ctx.editor.SwitchTo(0); + } + const Buffer *cur = ctx.editor.CurrentBuffer(); + ctx.editor.SetStatus( + std::string("Closed: ") + name_close + std::string( + " Now: ") + + (cur ? buffer_display_name(*cur) : std::string(""))); + } + // Close the prompt after successful confirmation + ctx.editor.CancelPrompt(); + } + } else { + ctx.editor.SetStatus("Save canceled"); + // Close the prompt after negative confirmation + ctx.editor.CancelPrompt(); + } + ctx.editor.ClearPendingOverwritePath(); + // Regardless of answer, end any close-after-save pending state for safety. + ctx.editor.SetCloseAfterSave(false); } else if (ctx.editor.CloseConfirmPending() && buf) { bool yes = false; if (!value.empty()) { @@ -2718,18 +2735,20 @@ cmd_newline(CommandContext &ctx) ctx.editor.SetSearchIndex(-1); return true; } - auto &rows = buf->Rows(); - std::size_t changed = 0; - for (auto &line: rows) { - std::string before = static_cast(line); - std::string after = std::regex_replace(before, rx, repl); - if (after != before) { - line = after; - ++changed; - } - } - buf->SetDirty(true); - ctx.editor.SetStatus("Regex replaced in " + std::to_string(changed) + " line(s)"); + std::size_t changed = 0; + // Iterate by index to allow modifications via PieceTable helpers + for (std::size_t y = 0; y < buf->Rows().size(); ++y) { + std::string before = static_cast(buf->Rows()[y]); + std::string after = std::regex_replace(before, rx, repl); + if (after != before) { + // Replace entire line y with 'after' using PieceTable ops + buf->delete_row(static_cast(y)); + buf->insert_row(static_cast(y), std::string_view(after)); + ++changed; + } + } + buf->SetDirty(true); + ctx.editor.SetStatus("Regex replaced in " + std::to_string(changed) + " line(s)"); // Clear search UI state ctx.editor.SetSearchActive(false); ctx.editor.SetSearchQuery(""); @@ -2753,38 +2772,29 @@ cmd_newline(CommandContext &ctx) ensure_cursor_visible(ctx.editor, *buf); return true; } - Buffer *buf = ctx.editor.CurrentBuffer(); - if (!buf) { - ctx.editor.SetStatus("No buffer to edit"); - return false; - } - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - int repeat = ctx.count > 0 ? ctx.count : 1; - for (int i = 0; i < repeat; ++i) { - if (y >= rows.size()) - rows.resize(y + 1); - auto &line = rows[y]; - std::string tail; - if (x < line.size()) { - tail = line.substr(x); - line.erase(x); - } - rows.insert(rows.begin() + static_cast(y + 1), Buffer::Line(tail)); - y += 1; - x = 0; - } - buf->SetCursor(x, y); - buf->SetDirty(true); - // Record newline after buffer modification; commit immediately for single-step undo - if (auto *u = buf->Undo()) { - u->Begin(UndoType::Newline); - u->commit(); - } - ensure_cursor_visible(ctx.editor, *buf); - return true; + Buffer *buf = ctx.editor.CurrentBuffer(); + if (!buf) { + ctx.editor.SetStatus("No buffer to edit"); + return false; + } + ensure_at_least_one_line(*buf); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + int repeat = ctx.count > 0 ? ctx.count : 1; + for (int i = 0; i < repeat; ++i) { + buf->split_line(static_cast(y), static_cast(x)); + // Move to start of next line + y += 1; + x = 0; + } + buf->SetCursor(x, y); + buf->SetDirty(true); + if (auto *u = buf->Undo()) { + u->Begin(UndoType::Newline); + u->commit(); + } + ensure_cursor_visible(ctx.editor, *buf); + return true; } @@ -2835,99 +2845,92 @@ cmd_backspace(CommandContext &ctx) } return true; } - Buffer *buf = ctx.editor.CurrentBuffer(); - if (!buf) { - ctx.editor.SetStatus("No buffer to edit"); - return false; - } - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - UndoSystem *u = buf->Undo(); - int repeat = ctx.count > 0 ? ctx.count : 1; - for (int i = 0; i < repeat; ++i) { - if (x > 0) { - // Delete character before cursor - char deleted = rows[y][x - 1]; - rows[y].erase(x - 1, 1); - --x; - // Update buffer cursor BEFORE Begin so batching sees correct cursor for backspace - buf->SetCursor(x, y); - // Record undo after deletion and cursor update - if (u) { - u->Begin(UndoType::Delete); - u->Append(deleted); - } - } else if (y > 0) { - // join with previous line - std::size_t prev_len = rows[y - 1].size(); - rows[y - 1] += rows[y]; - rows.erase(rows.begin() + static_cast(y)); - y = y - 1; - x = prev_len; - // Update cursor to the join point BEFORE Begin to keep invariants consistent - buf->SetCursor(x, y); - // Record a newline deletion that joined lines; commit immediately - if (u) { - u->Begin(UndoType::Newline); - u->commit(); - } - } else { - // at very start; nothing to do - break; - } - } - // Ensure buffer cursor reflects final x,y - buf->SetCursor(x, y); - buf->SetDirty(true); - ensure_cursor_visible(ctx.editor, *buf); - return true; + Buffer *buf = ctx.editor.CurrentBuffer(); + if (!buf) { + ctx.editor.SetStatus("No buffer to edit"); + return false; + } + ensure_at_least_one_line(*buf); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + UndoSystem *u = buf->Undo(); + int repeat = ctx.count > 0 ? ctx.count : 1; + for (int i = 0; i < repeat; ++i) { + // Refresh a read-only view of lines for char capture/lengths + const auto &rows_view = buf->Rows(); + if (x > 0) { + char deleted = '\0'; + if (y < rows_view.size() && x - 1 < rows_view[y].size()) + deleted = rows_view[y][x - 1]; + buf->delete_text(static_cast(y), static_cast(x - 1), 1); + x -= 1; + buf->SetCursor(x, y); + if (u) { + u->Begin(UndoType::Delete); + if (deleted != '\0') + u->Append(deleted); + } + } else if (y > 0) { + // Compute previous line length before join + std::size_t prev_len = 0; + if (y - 1 < rows_view.size()) + prev_len = rows_view[y - 1].size(); + buf->join_lines(static_cast(y - 1)); + y = y - 1; + x = prev_len; + buf->SetCursor(x, y); + if (u) { + u->Begin(UndoType::Newline); + u->commit(); + } + } else { + break; + } + } + buf->SetCursor(x, y); + buf->SetDirty(true); + ensure_cursor_visible(ctx.editor, *buf); + return true; } static bool cmd_delete_char(CommandContext &ctx) { - Buffer *buf = ctx.editor.CurrentBuffer(); - if (!buf) { - ctx.editor.SetStatus("No buffer to edit"); - return false; - } - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - UndoSystem *u = buf->Undo(); - int repeat = ctx.count > 0 ? ctx.count : 1; - for (int i = 0; i < repeat; ++i) { - if (y >= rows.size()) - break; - if (x < rows[y].size()) { - // Forward delete at cursor - char deleted = rows[y][x]; - rows[y].erase(x, 1); - // Record undo after deletion (cursor stays at same position) - if (u) { - u->Begin(UndoType::Delete); - u->Append(deleted); - } - } else if (y + 1 < rows.size()) { - // join next line - rows[y] += rows[y + 1]; - rows.erase(rows.begin() + static_cast(y + 1)); - // Record newline deletion at end of this line; commit immediately - if (u) { - u->Begin(UndoType::Newline); - u->commit(); - } - } else { - break; - } - } - buf->SetDirty(true); - ensure_cursor_visible(ctx.editor, *buf); - return true; + Buffer *buf = ctx.editor.CurrentBuffer(); + if (!buf) { + ctx.editor.SetStatus("No buffer to edit"); + return false; + } + ensure_at_least_one_line(*buf); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + UndoSystem *u = buf->Undo(); + int repeat = ctx.count > 0 ? ctx.count : 1; + for (int i = 0; i < repeat; ++i) { + const auto &rows_view = buf->Rows(); + if (y >= rows_view.size()) + break; + if (x < rows_view[y].size()) { + char deleted = rows_view[y][x]; + buf->delete_text(static_cast(y), static_cast(x), 1); + if (u) { + u->Begin(UndoType::Delete); + u->Append(deleted); + } + } else if (y + 1 < rows_view.size()) { + buf->join_lines(static_cast(y)); + if (u) { + u->Begin(UndoType::Newline); + u->commit(); + } + } else { + break; + } + } + buf->SetDirty(true); + ensure_cursor_visible(ctx.editor, *buf); + return true; } @@ -2972,95 +2975,98 @@ cmd_redo(CommandContext &ctx) static bool cmd_kill_to_eol(CommandContext &ctx) { - Buffer *buf = ctx.editor.CurrentBuffer(); - if (!buf) { - ctx.editor.SetStatus("No buffer to edit"); - return false; - } - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - int repeat = ctx.count > 0 ? ctx.count : 1; - std::string killed_total; - for (int i = 0; i < repeat; ++i) { - if (y >= rows.size()) - break; - if (x < rows[y].size()) { - // delete from cursor to end of line - killed_total += rows[y].substr(x); - rows[y].erase(x); - } else if (y + 1 < rows.size()) { - // at EOL: delete the newline (join with next line) - killed_total += "\n"; - rows[y] += rows[y + 1]; - rows.erase(rows.begin() + static_cast(y + 1)); - } else { - // nothing to delete - break; - } - } - buf->SetDirty(true); - ensure_cursor_visible(ctx.editor, *buf); - if (!killed_total.empty()) { - if (ctx.editor.KillChain()) - ctx.editor.KillRingAppend(killed_total); - else - ctx.editor.KillRingPush(killed_total); - ctx.editor.SetKillChain(true); - } - return true; + Buffer *buf = ctx.editor.CurrentBuffer(); + if (!buf) { + ctx.editor.SetStatus("No buffer to edit"); + return false; + } + ensure_at_least_one_line(*buf); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + int repeat = ctx.count > 0 ? ctx.count : 1; + std::string killed_total; + for (int i = 0; i < repeat; ++i) { + const auto &rows_view = buf->Rows(); + if (y >= rows_view.size()) + break; + if (x < rows_view[y].size()) { + // delete from cursor to end of line + killed_total += rows_view[y].substr(x); + std::size_t len = rows_view[y].size() - x; + buf->delete_text(static_cast(y), static_cast(x), len); + } else if (y + 1 < rows_view.size()) { + // at EOL: delete the newline (join with next line) + killed_total += "\n"; + buf->join_lines(static_cast(y)); + } else { + // nothing to delete + break; + } + } + buf->SetDirty(true); + ensure_cursor_visible(ctx.editor, *buf); + if (!killed_total.empty()) { + if (ctx.editor.KillChain()) + ctx.editor.KillRingAppend(killed_total); + else + ctx.editor.KillRingPush(killed_total); + ctx.editor.SetKillChain(true); + } + return true; } static bool cmd_kill_line(CommandContext &ctx) { - Buffer *buf = ctx.editor.CurrentBuffer(); - if (!buf) { - ctx.editor.SetStatus("No buffer to edit"); - return false; - } - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - (void) x; // cursor x will be reset to 0 - int repeat = ctx.count > 0 ? ctx.count : 1; - std::string killed_total; - for (int i = 0; i < repeat; ++i) { - if (rows.empty()) - break; - if (rows.size() == 1) { - // last remaining line: clear its contents - killed_total += static_cast(rows[0]); - rows[0].Clear(); - y = 0; - } else if (y < rows.size()) { - // erase current line; keep y pointing at the next line - killed_total += static_cast(rows[y]); - killed_total += "\n"; - rows.erase(rows.begin() + static_cast(y)); - if (y >= rows.size()) { - // deleted last line; move to previous - y = rows.size() - 1; - } - } else { - // out of range - y = rows.empty() ? 0 : rows.size() - 1; - } - } - buf->SetCursor(0, y); - buf->SetDirty(true); - ensure_cursor_visible(ctx.editor, *buf); - if (!killed_total.empty()) { - if (ctx.editor.KillChain()) - ctx.editor.KillRingAppend(killed_total); - else - ctx.editor.KillRingPush(killed_total); - ctx.editor.SetKillChain(true); - } - return true; + Buffer *buf = ctx.editor.CurrentBuffer(); + if (!buf) { + ctx.editor.SetStatus("No buffer to edit"); + return false; + } + ensure_at_least_one_line(*buf); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + (void) x; // cursor x will be reset to 0 + int repeat = ctx.count > 0 ? ctx.count : 1; + std::string killed_total; + for (int i = 0; i < repeat; ++i) { + const auto &rows_view = buf->Rows(); + if (rows_view.empty()) + break; + if (rows_view.size() == 1) { + // last remaining line: clear its contents + killed_total += static_cast(rows_view[0]); + if (!rows_view[0].empty()) + buf->delete_text(0, 0, rows_view[0].size()); + y = 0; + } else if (y < rows_view.size()) { + // erase current line; keep y pointing at the next line + killed_total += static_cast(rows_view[y]); + killed_total += "\n"; + buf->delete_row(static_cast(y)); + const auto &rows_after = buf->Rows(); + if (y >= rows_after.size()) { + // deleted last line; move to previous + y = rows_after.empty() ? 0 : rows_after.size() - 1; + } + } else { + // out of range + const auto &rows2 = buf->Rows(); + y = rows2.empty() ? 0 : rows2.size() - 1; + } + } + buf->SetCursor(0, y); + buf->SetDirty(true); + ensure_cursor_visible(ctx.editor, *buf); + if (!killed_total.empty()) { + if (ctx.editor.KillChain()) + ctx.editor.KillRingAppend(killed_total); + else + ctx.editor.KillRingPush(killed_total); + ctx.editor.SetKillChain(true); + } + return true; } @@ -3109,8 +3115,8 @@ cmd_move_file_end(CommandContext &ctx) Buffer *buf = ctx.editor.CurrentBuffer(); if (!buf) return false; - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + ensure_at_least_one_line(*buf); + const auto &rows = buf->Rows(); std::size_t y = rows.empty() ? 0 : rows.size() - 1; std::size_t x = rows.empty() ? 0 : rows[y].size(); buf->SetCursor(x, y); @@ -3809,38 +3815,13 @@ cmd_delete_word_prev(CommandContext &ctx) break; --x; } - // Now delete from (x, y) to (start_x, start_y) - std::string deleted; - if (y == start_y) { - // same line - if (x < start_x) { - deleted = rows[y].substr(x, start_x - x); - rows[y].erase(x, start_x - x); - } - } else { - // spans multiple lines - // First, collect text from (x, y) to end of line y - deleted = rows[y].substr(x); - rows[y].erase(x); - // Then collect complete lines between y and start_y - for (std::size_t ly = y + 1; ly < start_y; ++ly) { - deleted += "\n"; - deleted += static_cast(rows[ly]); - } - // Finally, collect from beginning of start_y to start_x - if (start_y < rows.size()) { - deleted += "\n"; - deleted += rows[start_y].substr(0, start_x); - rows[y] += rows[start_y].substr(start_x); - // Remove lines from y+1 to start_y inclusive - rows.erase(rows.begin() + static_cast(y + 1), - rows.begin() + static_cast(start_y + 1)); - } - } - // Prepend to killed_total (since we're deleting backwards) - killed_total = deleted + killed_total; - } - buf->SetCursor(x, y); + // Now delete from (x, y) to (start_x, start_y) using PieceTable + std::string deleted = extract_region_text(*buf, x, y, start_x, start_y); + delete_region(*buf, x, y, start_x, start_y); + // Prepend to killed_total (since we're deleting backwards) + killed_total = deleted + killed_total; + } + buf->SetCursor(x, y); buf->SetDirty(true); ensure_cursor_visible(ctx.editor, *buf); if (!killed_total.empty()) { @@ -3857,22 +3838,22 @@ cmd_delete_word_prev(CommandContext &ctx) static bool cmd_delete_word_next(CommandContext &ctx) { - Buffer *buf = ctx.editor.CurrentBuffer(); - if (!buf) - return false; - if (auto *u = buf->Undo()) - u->commit(); - ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); - std::size_t y = buf->Cury(); - std::size_t x = buf->Curx(); - int repeat = ctx.count > 0 ? ctx.count : 1; - std::string killed_total; - for (int i = 0; i < repeat; ++i) { - if (y >= rows.size()) - break; - std::size_t start_y = y; - std::size_t start_x = x; + Buffer *buf = ctx.editor.CurrentBuffer(); + if (!buf) + return false; + if (auto *u = buf->Undo()) + u->commit(); + ensure_at_least_one_line(*buf); + const auto &rows = buf->Rows(); + std::size_t y = buf->Cury(); + std::size_t x = buf->Curx(); + int repeat = ctx.count > 0 ? ctx.count : 1; + std::string killed_total; + for (int i = 0; i < repeat; ++i) { + if (y >= rows.size()) + break; + std::size_t start_y = y; + std::size_t start_x = x; // First, if currently on a word, skip to its end while (y < rows.size()) { if (x < rows[y].size() && is_word_char(static_cast(rows[y][x]))) { @@ -3905,42 +3886,16 @@ cmd_delete_word_next(CommandContext &ctx) continue; } } - // Now delete from (start_x, start_y) to (x, y) - std::string deleted; - if (start_y == y) { - // same line - if (start_x < x) { - deleted = rows[y].substr(start_x, x - start_x); - rows[y].erase(start_x, x - start_x); - x = start_x; - } - } else { - // spans multiple lines - // First, collect text from start_x to end of line start_y - deleted = rows[start_y].substr(start_x); - rows[start_y].erase(start_x); - // Then collect complete lines between start_y and y - for (std::size_t ly = start_y + 1; ly < y; ++ly) { - deleted += "\n"; - deleted += static_cast(rows[ly]); - } - // Finally, collect from beginning of y to x - if (y < rows.size()) { - deleted += "\n"; - deleted += rows[y].substr(0, x); - rows[start_y] += rows[y].substr(x); - // Remove lines from start_y+1 to y inclusive - rows.erase(rows.begin() + static_cast(start_y + 1), - rows.begin() + static_cast(y + 1)); - } - y = start_y; - x = start_x; - } - killed_total += deleted; - } - buf->SetCursor(x, y); - buf->SetDirty(true); - ensure_cursor_visible(ctx.editor, *buf); + // Now delete from (start_x, start_y) to (x, y) using PieceTable + std::string deleted = extract_region_text(*buf, start_x, start_y, x, y); + delete_region(*buf, start_x, start_y, x, y); + y = start_y; + x = start_x; + killed_total += deleted; + } + buf->SetCursor(x, y); + buf->SetDirty(true); + ensure_cursor_visible(ctx.editor, *buf); if (!killed_total.empty()) { if (ctx.editor.KillChain()) ctx.editor.KillRingAppend(killed_total); @@ -3967,14 +3922,13 @@ cmd_indent_region(CommandContext &ctx) ctx.editor.SetStatus("No region to indent"); return false; } - auto &rows = buf->Rows(); - for (std::size_t y = sy; y <= ey && y < rows.size(); ++y) { - rows[y].insert(0, "\t"); - } - buf->SetDirty(true); - buf->ClearMark(); - ensure_cursor_visible(ctx.editor, *buf); - return true; + for (std::size_t y = sy; y <= ey && y < buf->Rows().size(); ++y) { + buf->insert_text(static_cast(y), 0, std::string_view("\t")); + } + buf->SetDirty(true); + buf->ClearMark(); + ensure_cursor_visible(ctx.editor, *buf); + return true; } @@ -3993,26 +3947,28 @@ cmd_unindent_region(CommandContext &ctx) ctx.editor.SetStatus("No region to unindent"); return false; } - auto &rows = buf->Rows(); - for (std::size_t y = sy; y <= ey && y < rows.size(); ++y) { - auto &line = rows[y]; - if (!line.empty()) { - if (line[0] == '\t') { - line.erase(0, 1); - } else if (line[0] == ' ') { - std::size_t spaces = 0; - while (spaces < line.size() && spaces < 8 && line[spaces] == ' ') { - ++spaces; - } - if (spaces > 0) - line.erase(0, spaces); - } - } - } - buf->SetDirty(true); - buf->ClearMark(); - ensure_cursor_visible(ctx.editor, *buf); - return true; + for (std::size_t y = sy; y <= ey && y < buf->Rows().size(); ++y) { + const auto &rows_view = buf->Rows(); + if (y >= rows_view.size()) + break; + const std::string line = static_cast(rows_view[y]); + if (!line.empty()) { + if (line[0] == '\t') { + buf->delete_text(static_cast(y), 0, 1); + } else if (line[0] == ' ') { + std::size_t spaces = 0; + while (spaces < line.size() && spaces < 8 && line[spaces] == ' ') { + ++spaces; + } + if (spaces > 0) + buf->delete_text(static_cast(y), 0, spaces); + } + } + } + buf->SetDirty(true); + buf->ClearMark(); + ensure_cursor_visible(ctx.editor, *buf); + return true; } @@ -4023,7 +3979,7 @@ cmd_reflow_paragraph(CommandContext &ctx) if (!buf) return false; ensure_at_least_one_line(*buf); - auto &rows = buf->Rows(); + auto &rows = buf->Rows(); std::size_t y = buf->Cury(); // Treat a universal-argument count of 1 as "no width specified". // Editor::UArgGet() returns 1 when no explicit count was provided. @@ -4243,10 +4199,17 @@ cmd_reflow_paragraph(CommandContext &ctx) if (new_lines.empty()) new_lines.push_back(""); - rows.erase(rows.begin() + static_cast(para_start), - rows.begin() + static_cast(para_end + 1)); - rows.insert(rows.begin() + static_cast(para_start), - new_lines.begin(), new_lines.end()); + // Replace paragraph lines via PieceTable-backed operations + for (std::size_t i = para_end; i + 1 > para_start; --i) { + buf->delete_row(static_cast(i)); + if (i == 0) break; // prevent wrap on size_t + } + // Insert new lines starting at para_start + std::size_t insert_y = para_start; + for (const auto &ln : new_lines) { + buf->insert_row(static_cast(insert_y), std::string_view(ln)); + insert_y += 1; + } // Place cursor at the end of the paragraph std::size_t new_last_y = para_start + (new_lines.empty() ? 0 : new_lines.size() - 1); diff --git a/Editor.cc b/Editor.cc index 87b4362..91af31b 100644 --- a/Editor.cc +++ b/Editor.cc @@ -197,9 +197,11 @@ Editor::OpenFile(const std::string &path, std::string &err) eng->InvalidateFrom(0); } } - return true; - } - } + // Defensive: ensure any active prompt is closed after a successful open + CancelPrompt(); + return true; + } + } Buffer b; if (!b.OpenFromFile(path, err)) { @@ -237,8 +239,10 @@ Editor::OpenFile(const std::string &path, std::string &err) } // Add as a new buffer and switch to it std::size_t idx = AddBuffer(std::move(b)); - SwitchTo(idx); - return true; + SwitchTo(idx); + // Defensive: ensure any active prompt is closed after a successful open + CancelPrompt(); + return true; } diff --git a/PieceTable.cc b/PieceTable.cc index 89c3194..f99de78 100644 --- a/PieceTable.cc +++ b/PieceTable.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include "PieceTable.h" @@ -757,3 +758,17 @@ PieceTable::Find(const std::string &needle, std::size_t start) const find_cache_.result = pos; return pos; } + + +void +PieceTable::WriteToStream(std::ostream &out) const +{ + // Stream the content piece-by-piece without forcing full materialization + for (const auto &p : pieces_) { + if (p.len == 0) + continue; + const std::string &src = (p.src == Source::Original) ? original_ : add_; + const char *base = src.data() + static_cast(p.start); + out.write(base, static_cast(p.len)); + } +} diff --git a/PieceTable.h b/PieceTable.h index 956a1f1..50218c8 100644 --- a/PieceTable.h +++ b/PieceTable.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -100,6 +101,9 @@ public: // Simple search utility; returns byte offset or npos [[nodiscard]] std::size_t Find(const std::string &needle, std::size_t start = 0) const; + // Stream out content without materializing the entire buffer + void WriteToStream(std::ostream &out) const; + // Heuristic configuration void SetConsolidationParams(std::size_t piece_limit, std::size_t small_piece_threshold, diff --git a/test_buffer_open_nonexistent_save.cc b/test_buffer_open_nonexistent_save.cc new file mode 100644 index 0000000..64b7c34 --- /dev/null +++ b/test_buffer_open_nonexistent_save.cc @@ -0,0 +1,50 @@ +// Test: Open a non-existent path (buffer becomes unnamed but with filename), +// insert text, then Save via SaveAs to the same path. Verify bytes on disk. +#include +#include +#include +#include + +#include "Buffer.h" + +static std::string read_file(const std::string &path) +{ + std::ifstream in(path, std::ios::binary); + return std::string((std::istreambuf_iterator(in)), std::istreambuf_iterator()); +} + +int main() +{ + const std::string path = "./.kte_test_open_nonexistent_save.tmp"; + std::remove(path.c_str()); + + // Sanity: path should not exist + { + std::ifstream probe(path); + assert(!probe.good()); + } + + Buffer buf; + std::string err; + bool ok = buf.OpenFromFile(path, err); + assert(ok && err.empty()); + assert(!buf.IsFileBacked()); + assert(buf.Filename().size() > 0); + + // Insert text like a user would type then press Return + buf.insert_text(0, 0, std::string("hello, world")); + // Simulate pressing Return (newline at end) + buf.insert_text(0, 12, std::string("\n")); + buf.SetDirty(true); + + // Save using SaveAs to the same filename the buffer carries (what cmd_save would do) + ok = buf.SaveAs(buf.Filename(), err); + assert(ok && err.empty()); + + const std::string got = read_file(buf.Filename()); + const std::string expected = std::string("hello, world\n"); + assert(got == expected); + + std::remove(path.c_str()); + return 0; +} diff --git a/test_buffer_save.cc b/test_buffer_save.cc new file mode 100644 index 0000000..85f5d0b --- /dev/null +++ b/test_buffer_save.cc @@ -0,0 +1,57 @@ +// Test that Buffer::Save writes actual contents to disk +#include +#include +#include +#include + +#include "Buffer.h" + +static std::string +read_file(const std::string &path) +{ + std::ifstream in(path, std::ios::binary); + return std::string((std::istreambuf_iterator(in)), std::istreambuf_iterator()); +} + +int +main() +{ + // Create a temporary path under current working directory + const std::string path = "./.kte_test_buffer_save.tmp"; + + // Ensure any previous file is removed + std::remove(path.c_str()); + + Buffer buf; + + // Simulate editing a new buffer: insert content + const std::string payload = "Hello, world!\nThis is a save test.\n"; + buf.insert_text(0, 0, payload); + + // Make it file-backed with a filename so Save() path is exercised + // We use SaveAs first to set filename/is_file_backed and then modify content + std::string err; + bool ok = buf.SaveAs(path, err); + assert(ok && err.empty()); + + // Modify buffer after SaveAs to ensure Save() writes new content + const std::string more = "Appended line.\n"; + buf.insert_text(2, 0, more); + + // Mark as dirty so commands would attempt to save; Save() itself doesn’t require dirty + buf.SetDirty(true); + + // Now call Save() which should use streaming write and persist all bytes + err.clear(); + ok = buf.Save(err); + assert(ok && err.empty()); + + // Verify file contents exactly match expected string + const std::string expected = payload + more; + const std::string got = read_file(path); + assert(got == expected); + + // Cleanup + std::remove(path.c_str()); + return 0; +} diff --git a/test_buffer_save_existing.cc b/test_buffer_save_existing.cc new file mode 100644 index 0000000..702cb0f --- /dev/null +++ b/test_buffer_save_existing.cc @@ -0,0 +1,48 @@ +// Test saving after opening an existing file +#include +#include +#include +#include + +#include "Buffer.h" + +static void write_file(const std::string &path, const std::string &data) +{ + std::ofstream out(path, std::ios::binary | std::ios::trunc); + out.write(data.data(), static_cast(data.size())); +} + +static std::string read_file(const std::string &path) +{ + std::ifstream in(path, std::ios::binary); + return std::string((std::istreambuf_iterator(in)), std::istreambuf_iterator()); +} + +int main() +{ + const std::string path = "./.kte_test_buffer_save_existing.tmp"; + std::remove(path.c_str()); + const std::string initial = "abc\n123\n"; + write_file(path, initial); + + Buffer buf; + std::string err; + bool ok = buf.OpenFromFile(path, err); + assert(ok && err.empty()); + + // Insert at end + buf.insert_text(2, 0, std::string("tail\n")); + buf.SetDirty(true); + + // Save should overwrite the same file + err.clear(); + ok = buf.Save(err); + assert(ok && err.empty()); + + const std::string expected = initial + "tail\n"; + const std::string got = read_file(path); + assert(got == expected); + + std::remove(path.c_str()); + return 0; +}