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.
This commit is contained in:
2025-12-07 00:30:11 -08:00
parent 657c9bbc19
commit f6f0c11be4
10 changed files with 682 additions and 498 deletions

View File

@@ -1,5 +1,6 @@
<component name="ProjectCodeStyleConfiguration"> <component name="ProjectCodeStyleConfiguration">
<state> <state>
<option name="USE_PER_PROJECT_SETTINGS" value="true" />
<option name="PREFERRED_PROJECT_CODE_STYLE" value="sccl" /> <option name="PREFERRED_PROJECT_CODE_STYLE" value="sccl" />
</state> </state>
</component> </component>

View File

@@ -292,28 +292,29 @@ Buffer::OpenFromFile(const std::string &path, std::string &err)
bool bool
Buffer::Save(std::string &err) const Buffer::Save(std::string &err) const
{ {
if (!is_file_backed_ || filename_.empty()) { if (!is_file_backed_ || filename_.empty()) {
err = "Buffer is not file-backed; use SaveAs()"; err = "Buffer is not file-backed; use SaveAs()";
return false; return false;
} }
std::ofstream out(filename_, std::ios::out | std::ios::binary | std::ios::trunc); std::ofstream out(filename_, std::ios::out | std::ios::binary | std::ios::trunc);
if (!out) { if (!out) {
err = "Failed to open for write: " + filename_ + ". Error: " + std::string(std::strerror(errno)); err = "Failed to open for write: " + filename_ + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
// Write the entire buffer in a single block to minimize I/O calls. // Stream the content directly from the piece table to avoid relying on
const char *data = content_.Data(); // full materialization, which may yield an empty pointer when size > 0.
const auto size = static_cast<std::streamsize>(content_.Size()); if (content_.Size() > 0) {
if (data != nullptr && size > 0) { content_.WriteToStream(out);
out.write(data, size); }
} // Ensure data hits the OS buffers
if (!out.good()) { out.flush();
err = "Write error: " + filename_ + ". Error: " + std::string(std::strerror(errno)); if (!out.good()) {
return false; 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. // Note: const method cannot change dirty_. Intentionally const to allow UI code
return true; // 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)); err = "Failed to open for write: " + out_path + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
// Write whole content in a single I/O operation // Stream content without forcing full materialization
const char *data = content_.Data(); if (content_.Size() > 0) {
const auto size = static_cast<std::streamsize>(content_.Size()); content_.WriteToStream(out);
if (data != nullptr && size > 0) { }
out.write(data, size); // Ensure data hits the OS buffers
} out.flush();
if (!out.good()) { if (!out.good()) {
err = "Write error: " + out_path + ". Error: " + std::string(std::strerror(errno)); err = "Write error: " + out_path + ". Error: " + std::string(std::strerror(errno));
return false; return false;
} }
filename_ = out_path; filename_ = out_path;
is_file_backed_ = true; is_file_backed_ = true;

View File

@@ -313,6 +313,47 @@ if (BUILD_TESTS)
target_link_libraries(test_undo ${TREESITTER_LIBRARY}) target_link_libraries(test_undo ${TREESITTER_LIBRARY})
endif () endif ()
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 () endif ()
if (${BUILD_GUI}) if (${BUILD_GUI})

File diff suppressed because it is too large Load Diff

View File

@@ -197,9 +197,11 @@ Editor::OpenFile(const std::string &path, std::string &err)
eng->InvalidateFrom(0); eng->InvalidateFrom(0);
} }
} }
return true; // Defensive: ensure any active prompt is closed after a successful open
} CancelPrompt();
} return true;
}
}
Buffer b; Buffer b;
if (!b.OpenFromFile(path, err)) { 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 // Add as a new buffer and switch to it
std::size_t idx = AddBuffer(std::move(b)); std::size_t idx = AddBuffer(std::move(b));
SwitchTo(idx); SwitchTo(idx);
return true; // Defensive: ensure any active prompt is closed after a successful open
CancelPrompt();
return true;
} }

View File

@@ -1,6 +1,7 @@
#include <algorithm> #include <algorithm>
#include <utility> #include <utility>
#include <limits> #include <limits>
#include <ostream>
#include "PieceTable.h" #include "PieceTable.h"
@@ -757,3 +758,17 @@ PieceTable::Find(const std::string &needle, std::size_t start) const
find_cache_.result = pos; find_cache_.result = pos;
return 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<std::ptrdiff_t>(p.start);
out.write(base, static_cast<std::streamsize>(p.len));
}
}

View File

@@ -5,6 +5,7 @@
#include <cstddef> #include <cstddef>
#include <cstdint> #include <cstdint>
#include <string> #include <string>
#include <ostream>
#include <vector> #include <vector>
#include <limits> #include <limits>
@@ -100,6 +101,9 @@ public:
// Simple search utility; returns byte offset or npos // Simple search utility; returns byte offset or npos
[[nodiscard]] std::size_t Find(const std::string &needle, std::size_t start = 0) const; [[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 // Heuristic configuration
void SetConsolidationParams(std::size_t piece_limit, void SetConsolidationParams(std::size_t piece_limit,
std::size_t small_piece_threshold, std::size_t small_piece_threshold,

View File

@@ -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 <cassert>
#include <cstdio>
#include <fstream>
#include <string>
#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<char>(in)), std::istreambuf_iterator<char>());
}
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;
}

57
test_buffer_save.cc Normal file
View File

@@ -0,0 +1,57 @@
// Test that Buffer::Save writes actual contents to disk
#include <cassert>
#include <cstdio>
#include <fstream>
#include <string>
#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<char>(in)), std::istreambuf_iterator<char>());
}
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 doesnt 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;
}

View File

@@ -0,0 +1,48 @@
// Test saving after opening an existing file
#include <cassert>
#include <cstdio>
#include <fstream>
#include <string>
#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<std::streamsize>(data.size()));
}
static std::string read_file(const std::string &path)
{
std::ifstream in(path, std::ios::binary);
return std::string((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
}
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;
}