Skip to content

fix(entry): guard LOCAL buffers from force-destroy in FileEntry:destroy - #318

Merged
dlyongemallo merged 1 commit into
mainfrom
fix_local_buffer_force_destroy
Sep 6, 2026
Merged

fix(entry): guard LOCAL buffers from force-destroy in FileEntry:destroy#318
dlyongemallo merged 1 commit into
mainfrom
fix_local_buffer_force_destroy

Conversation

@dlyongemallo

@dlyongemallo dlyongemallo commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Motivated by #315 (comment).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is small, correctly scoped to guard LOCAL buffers, consistent with the existing created_bufs cleanup design, and fully covered by accurate tests with no issues found.

Pull request overview

This PR hardens buffer lifecycle handling in the diff scene by preventing FileEntry:destroy from force-deleting the user's real working-tree (LOCAL) buffers. Previously, refresh_files({ force = true }) (which reaches old_file:destroy(true) at diff_view.lua:860) would propagate force=true to every side of an entry, unconditionally deleting even LOCAL buffers — collapsing the tab the user opened from and losing its cursor position. The new force_for helper keeps forcing STAGE/COMMIT/CUSTOM virtual buffers (so their throwaway edits are still discarded) while downgrading LOCAL sides to the guarded force=false path. This is consistent with the existing separate cleanup of diffview-created LOCAL buffers via File.created_bufs in close().

Changes:

  • Added a force_for(f) helper in FileEntry:destroy that only forwards force=true to non-LOCAL rev sides.
  • Documented the rationale for guarding LOCAL buffers.
  • Extended the destroy tests to cover mixed STAGE/LOCAL/COMMIT sides and the force=false pass-through case.
File summaries
File Description
lua/diffview/scene/file_entry.lua Introduces force_for so LOCAL files are never force-destroyed, with an explanatory doc comment.
lua/diffview/tests/functional/file_entry_spec.lua Updates/adds tests asserting force is guarded for LOCAL and passed through unchanged when force=false.

I verified that the only destroy(true) caller is the force-refresh replace path (diff_view.lua:860), that RevType values match, that the helper logic and both tests are correct, and that LOCAL buffers created by diffview are still reaped separately via File.created_bufs, so this change does not leak buffers.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dlyongemallo
dlyongemallo merged commit 7e0d73d into main Sep 6, 2026
9 checks passed
@dlyongemallo
dlyongemallo deleted the fix_local_buffer_force_destroy branch September 6, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants