Guard data-loss paths: quit, reload, close, recovery answers, save-as

- Quit checks every buffer for unsaved changes and names them; it used
  to look only at the current buffer and exit, dropping the rest (an
  unnamed buffer's text was unrecoverable). Save-and-quit does not quit
  while other buffers are dirty.
- Reload of a dirty buffer needs a second C-k l with no edit in between
  (it discarded edits, undo history and the journal at once), and a file
  deleted on disk is not 'reloaded' into an empty buffer.
- At the destructive y/n prompts ("Recover swap edits?" and "Save
  changes to X?"), only an explicit 'n' discards; Enter or any other key
  cancels. Keys typed while the startup recovery prompt appeared used to
  delete the crash journal. SwapRecoveryPrompt_Discard now answers 'n'.
- Saving from the close prompt refuses to overwrite a file changed on
  disk, like save.
- Save-as onto a file open in another buffer is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
Claude
2026-09-24 09:16:04 -07:00
committed by Kyle Isom
parent fb50a02f6a
commit 6c2727789e
4 changed files with 207 additions and 15 deletions
+98 -13
View File
@@ -363,6 +363,17 @@ safe_current_path()
}
// True if `path` is open in a buffer other than the current one. Saving the
// current buffer there would leave two buffers for one file (sharing, and
// deleting each other's, swap journal).
static bool
open_in_other_buffer(const Editor &ed, const std::string &path)
{
const std::size_t idx = ed.FindOpenBuffer(path);
return idx != static_cast<std::size_t>(-1) && idx != ed.CurrentBufferIndex();
}
// True if `path` names the file `buf` is backed by (same inode).
static bool
is_buffers_own_file(const Buffer &buf, const std::string &path)
@@ -1018,6 +1029,10 @@ cmd_save_as(CommandContext &ctx)
ctx.editor.SetStatus("save-as requires a filename");
return false;
}
if (open_in_other_buffer(ctx.editor, ctx.arg)) {
ctx.editor.SetStatus(ctx.arg + " is open in another buffer; save or close that buffer instead");
return false;
}
// Ask before replacing an existing file other than the buffer's own.
if (fs_exists(ctx.arg) && !is_buffers_own_file(*buf, ctx.arg)) {
ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", "");
@@ -1041,6 +1056,31 @@ cmd_save_as(CommandContext &ctx)
}
static std::string buffer_display_name(const Buffer &b);
// Display names of buffers with unsaved changes ("a.txt, b.txt and 2 more"),
// or empty if none.
static std::string
dirty_buffer_names(const Editor &ed)
{
std::vector<std::string> names;
for (const auto &b: ed.Buffers()) {
if (b.Dirty())
names.push_back(buffer_display_name(b));
}
std::string out;
for (std::size_t i = 0; i < names.size() && i < 3; ++i) {
if (i > 0)
out += ", ";
out += names[i];
}
if (names.size() > 3)
out += " and " + std::to_string(names.size() - 3) + " more";
return out;
}
static bool
cmd_quit(CommandContext &ctx)
{
@@ -1052,9 +1092,12 @@ cmd_quit(CommandContext &ctx)
ctx.editor.SetStatus("Quit requested");
return true;
}
// If current buffer exists and is dirty, warn and arm confirmation
if (buf && buf->Dirty()) {
ctx.editor.SetStatus("Unsaved changes. C-k q to quit without saving");
(void) buf;
// Any dirty buffer (not only the current one) needs confirmation: quitting
// used to drop other buffers' changes without a word.
const std::string dirty = dirty_buffer_names(ctx.editor);
if (!dirty.empty()) {
ctx.editor.SetStatus("Unsaved changes in " + dirty + ". C-k q again to quit without saving");
ctx.editor.SetQuitConfirmPending(true);
return true;
}
@@ -1101,6 +1144,12 @@ cmd_save_and_quit(CommandContext &ctx)
return false;
}
}
// Other buffers may still hold unsaved changes: do not drop them.
const std::string dirty = dirty_buffer_names(ctx.editor);
if (!dirty.empty()) {
ctx.editor.SetStatus("Saved; unsaved changes remain in " + dirty + ". Not quitting");
return true;
}
ctx.editor.SetStatus("Save and quit requested");
ctx.editor.SetQuitRequested(true);
return true;
@@ -1988,7 +2037,7 @@ cmd_buffer_close(const CommandContext &ctx)
if (b && b->Dirty()) {
ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Save", "");
ctx.editor.SetCloseConfirmPending(true);
ctx.editor.SetStatus(std::string("Save changes to ") + name + "? (y/N)");
ctx.editor.SetStatus(std::string("Save changes to ") + name + "? (y/n, C-g cancel)");
return true;
}
// Otherwise close immediately
@@ -2957,7 +3006,9 @@ cmd_newline(CommandContext &ctx)
value = expand_user_path(value);
// Ask before overwriting any existing file other than the
// buffer's own (it used to ask only for unnamed buffers).
if (fs_exists(value) && !is_buffers_own_file(*buf, value)) {
if (open_in_other_buffer(ctx.editor, value)) {
ctx.editor.SetStatus(value + " is open in another buffer; save or close that buffer instead");
} else if (fs_exists(value) && !is_buffers_own_file(*buf, value)) {
ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", "");
ctx.editor.SetPendingOverwritePath(value);
ctx.editor.SetStatus(
@@ -3061,20 +3112,38 @@ cmd_newline(CommandContext &ctx)
// Regardless of answer, end any close-after-save pending state for safety.
ctx.editor.SetCloseAfterSave(false);
} else if (ctx.editor.PendingRecoveryPrompt() != Editor::RecoveryPromptKind::None) {
bool yes = false;
if (!value.empty()) {
char c = value[0];
yes = (c == 'y' || c == 'Y');
const char c = value.empty() ? '\0' : value[0];
const bool yes = (c == 'y' || c == 'Y');
const bool no = (c == 'n' || c == 'N');
// "Discard" deletes the crash journal, so it takes an explicit
// 'n'. Enter or any other key (e.g. typing that was meant for the
// buffer when the prompt appeared) cancels, keeping the journal.
if (!yes && !no &&
ctx.editor.PendingRecoveryPrompt() == Editor::RecoveryPromptKind::RecoverOrDiscard) {
ctx.editor.CancelRecoveryPrompt();
ctx.editor.CancelPrompt();
ctx.editor.SetStatus("Recovery canceled; swap file kept (answer y or n)");
return true;
}
(void) ctx.editor.ResolveRecoveryPrompt(yes);
ctx.editor.CancelPrompt();
// Continue any queued opens (e.g., startup argv files).
ctx.editor.ProcessPendingOpens();
} else if (ctx.editor.CloseConfirmPending() && buf) {
bool yes = false;
if (!value.empty()) {
char c = value[0];
yes = (c == 'y' || c == 'Y');
const char c = value.empty() ? '\0' : value[0];
const bool yes = (c == 'y' || c == 'Y');
// Closing without saving discards the edits (and their journal),
// so it takes an explicit 'n'; anything else cancels the close.
if (!yes && c != 'n' && c != 'N') {
ctx.editor.SetCloseConfirmPending(false);
ctx.editor.SetStatus("Close canceled (answer y or n)");
return true;
}
// Saving here must not silently overwrite changes made on disk.
if (yes && buf->IsFileBacked() && buf->ExternallyModifiedOnDisk()) {
ctx.editor.SetCloseConfirmPending(false);
ctx.editor.SetStatus("File changed on disk; not saved or closed. Save with C-k s to confirm overwriting");
return true;
}
// Prepare close details
std::size_t idx_close = ctx.editor.CurrentBufferIndex();
@@ -5283,6 +5352,22 @@ cmd_reload_buffer(CommandContext &ctx)
ctx.editor.SetStatus("Cannot reload unnamed buffer");
return false;
}
// Reloading a file that is gone would replace the buffer with nothing.
if (!fs_exists(filename)) {
ctx.editor.SetStatus("File no longer exists on disk; not reloading " + filename);
return false;
}
// Reload discards unsaved edits (and their undo history and journal), so
// a dirty buffer needs a second C-k l with no edit in between.
static const Buffer *confirm_buf = nullptr;
static std::uint64_t confirm_version = 0;
if (buf->Dirty() && !(confirm_buf == buf && confirm_version == buf->Version())) {
confirm_buf = buf;
confirm_version = buf->Version();
ctx.editor.SetStatus("Unsaved changes will be lost. C-k l again to reload anyway");
return true;
}
confirm_buf = nullptr;
std::string err;
if (!buf->OpenFromFile(filename, err)) {
ctx.editor.SetStatus(std::string("Reload failed: ") + err);
+1 -1
View File
@@ -555,7 +555,7 @@ Editor::ProcessPendingOpens()
pending_recovery_open_ = req;
pending_recovery_swap_path_ = swp;
StartPrompt(PromptKind::Confirm, "Recover", "");
SetStatus("Recover swap edits for " + req.path + "? (y/N, C-g cancel)");
SetStatus("Recover swap edits for " + req.path + "? (y/n, C-g cancel)");
return opened_any;
}
} else {
+76
View File
@@ -898,3 +898,79 @@ TEST(Audit_ForEachRegexMatch_LongLine)
ASSERT_EQ(n, (std::size_t) 1);
ASSERT_EQ(total, line.size());
}
// Quit checks every buffer for unsaved changes, not just the current one.
TEST(Audit_Quit_ChecksAllBuffers)
{
TestHarness h;
Editor &ed = h.EditorRef();
h.Buf().insert_text(0, 0, "unsaved");
h.Buf().SetDirty(true);
Buffer other;
ed.AddBuffer(std::move(other));
ed.SwitchTo(ed.BufferCount() - 1);
ASSERT_TRUE(!ed.CurrentBuffer()->Dirty());
ASSERT_TRUE(h.Exec(CommandId::Quit));
ASSERT_TRUE(!ed.QuitRequested());
ASSERT_TRUE(h.Exec(CommandId::Quit));
ASSERT_TRUE(ed.QuitRequested());
}
// Reload refuses a file deleted on disk and asks before discarding edits.
TEST(Audit_Reload_ConfirmsAndRefusesDeletedFile)
{
TempDir d("reload_safety");
const std::string file = (d.path / "r.txt").string();
std::ofstream(file) << "disk\n";
TestHarness h;
Editor &ed = h.EditorRef();
std::string err;
ASSERT_TRUE(ed.OpenFile(file, err));
ASSERT_TRUE(h.Exec(CommandId::InsertText, "EDIT "));
ASSERT_TRUE(h.Exec(CommandId::ReloadBuffer));
ASSERT_EQ(h.Line(0), std::string("EDIT disk"));
std::filesystem::remove(file);
(void) h.Exec(CommandId::ReloadBuffer);
(void) h.Exec(CommandId::ReloadBuffer);
ASSERT_EQ(h.Line(0), std::string("EDIT disk"));
}
// Closing a dirty buffer: Enter alone at "Save changes?" cancels instead of
// discarding the edits.
TEST(Audit_CloseConfirm_EmptyAnswerCancels)
{
TestHarness h;
Editor &ed = h.EditorRef();
h.Buf().insert_text(0, 0, "work");
h.Buf().SetDirty(true);
const std::size_t n = ed.BufferCount();
ASSERT_TRUE(h.Exec(CommandId::BufferClose));
ASSERT_TRUE(ed.PromptActive());
ASSERT_TRUE(h.Exec(CommandId::Newline));
ASSERT_EQ(ed.BufferCount(), n);
ASSERT_EQ(h.Text(), std::string("work"));
}
// Save-as onto a file that is open in another buffer is refused (it would
// leave two buffers, and one journal, for the same file).
TEST(Audit_SaveAs_FileOpenInOtherBuffer_Refused)
{
TempDir d("saveas_open");
std::ofstream(d.path / "y.txt") << "y\n";
std::ofstream(d.path / "z.txt") << "z\n";
TestHarness h;
Editor &ed = h.EditorRef();
std::string err;
ASSERT_TRUE(ed.OpenFile((d.path / "y.txt").string(), err));
ASSERT_TRUE(ed.OpenFile((d.path / "z.txt").string(), err));
const std::size_t n = ed.BufferCount();
(void) Execute(ed, "save-as", (d.path / "y.txt").string());
ASSERT_TRUE(!ed.PromptActive());
ASSERT_EQ(slurp(d.path / "y.txt"), std::string("y\n"));
ASSERT_EQ(ed.BufferCount(), n);
ASSERT_EQ(ed.FindOpenBuffer((d.path / "y.txt").string()) != ed.CurrentBufferIndex(), true);
}
+32 -1
View File
@@ -165,7 +165,9 @@ TEST(SwapRecoveryPrompt_Discard_DeletesSwapAndOpensClean)
ASSERT_EQ(ed.PendingRecoveryPrompt(), Editor::RecoveryPromptKind::RecoverOrDiscard);
ASSERT_EQ(ed.PromptActive(), true);
// Default answer (empty) is 'no' => discard.
// An explicit 'n' discards. (An empty answer cancels instead; see
// SwapRecoveryPrompt_EmptyAnswer_KeepsSwap.)
ASSERT_TRUE(Execute(ed, CommandId::InsertText, "n"));
ASSERT_TRUE(Execute(ed, CommandId::Newline));
ASSERT_EQ(ed.PendingRecoveryPrompt(), Editor::RecoveryPromptKind::None);
ASSERT_EQ(ed.PromptActive(), false);
@@ -506,6 +508,9 @@ TEST(SwapJournal_Reload_ResetsJournal)
Buffer *cur = ed.CurrentBuffer();
cur->SetCursor(0, 0);
ASSERT_TRUE(Execute(ed, CommandId::InsertText, "XXXX"));
// The buffer is dirty: the first reload only asks for confirmation.
ASSERT_TRUE(Execute(ed, CommandId::ReloadBuffer));
ASSERT_TRUE(ed.CurrentBuffer()->Dirty());
ASSERT_TRUE(Execute(ed, CommandId::ReloadBuffer));
cur = ed.CurrentBuffer();
cur->SetCursor(0, 1);
@@ -622,3 +627,29 @@ TEST(SwapJournal_SecondSession_DoesNotDeleteOwnersJournal)
ASSERT_TRUE(kte::SwapManager::ReplayFile(check, swp, rerr));
ASSERT_EQ(buffer_bytes_via_views(check), std::string("Abase\n"));
}
// Enter alone (or any key other than y/n) at the recovery prompt cancels and
// keeps the journal: it used to count as "no" and delete it.
TEST(SwapRecoveryPrompt_EmptyAnswer_KeepsSwap)
{
ktet::InstallDefaultCommandsOnce();
XdgSandbox sb("empty_answer");
const std::string file = (sb.root / "work" / "e.txt").string();
write_file_bytes(file, "base\n");
std::string expected;
const std::string swp = make_journal(file, [](Buffer &b) {
b.insert_text(0, 0, std::string("A"));
}, expected);
Editor ed;
ed.SetDimensions(24, 80);
ed.AddBuffer(Buffer());
ed.RequestOpenFile(file);
(void) ed.ProcessPendingOpens();
ASSERT_EQ(ed.PendingRecoveryPrompt(), Editor::RecoveryPromptKind::RecoverOrDiscard);
answer(ed, "x");
ASSERT_EQ(ed.PendingRecoveryPrompt(), Editor::RecoveryPromptKind::None);
ASSERT_TRUE(!ed.PromptActive());
ASSERT_TRUE(std::filesystem::exists(swp));
}