Skip to content

fix: check fs_rename return value in selection store#103

Merged
dlyongemallo merged 1 commit intomainfrom
catch_rename_failures
Apr 13, 2026
Merged

fix: check fs_rename return value in selection store#103
dlyongemallo merged 1 commit intomainfrom
catch_rename_failures

Conversation

@dlyongemallo
Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings April 13, 2026 05:48
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves robustness of the selection persistence logic by detecting and surfacing failures during the atomic temp-file rename step, and adds a regression test to ensure failures are logged.

Changes:

  • Check and handle vim.uv.fs_rename() return value when writing the selection store.
  • Add a functional regression test asserting a warning is logged when saving fails (intended to cover rename failures).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
lua/diffview/selection_store.lua Adds error handling for failed fs_rename() during atomic store writes.
lua/diffview/tests/functional/selection_store_spec.lua Adds a regression test that expects a warning when a save operation fails (intended to cover rename failure).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lua/diffview/selection_store.lua
Comment thread lua/diffview/tests/functional/selection_store_spec.lua Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lua/diffview/selection_store.lua Outdated
Comment thread lua/diffview/tests/functional/selection_store_spec.lua Outdated
`vim.uv.fs_rename` returns nil+error on failure rather than throwing, so
the surrounding pcall was not catching rename failures. Explicitly check
the return value and raise on failure.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dlyongemallo dlyongemallo merged commit 9b8ab51 into main Apr 13, 2026
6 checks passed
@dlyongemallo dlyongemallo deleted the catch_rename_failures branch April 13, 2026 06:39
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