Skip to content

Align backup system to Neovim's system - #11374

Open
kirawi wants to merge 9 commits into
helix-editor:masterfrom
kirawi:open
Open

Align backup system to Neovim's system#11374
kirawi wants to merge 9 commits into
helix-editor:masterfrom
kirawi:open

Conversation

@kirawi

@kirawi kirawi commented Jul 30, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

kirawi
kirawi commented Jul 30, 2024
View reviewed changes
Comment thread helix-stdx/src/faccess.rs Outdated
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Jul 30, 2024
@kirawi
kirawi force-pushed the open branch 4 times, most recently from 0a9d853 to c80b4d7 Compare July 31, 2024 01:25
@kirawi

This comment was marked as outdated.

Sign in to view
@archseer archseer added this to the next milestone Aug 9, 2024
@kirawi kirawi mentioned this pull request Nov 10, 2024
Open
@kirawi

kirawi commented Nov 16, 2024

Copy link
Copy Markdown
Member Author

I'll refactor this pr per #12074

@kirawi kirawi changed the title Create temporary files with the same permissions as the original file Align backup system to Neovim's system Nov 17, 2024
@kirawi

This comment was marked as off-topic.

Sign in to view
@kirawi kirawi added the C-bug Category: This is a bug label Nov 17, 2024
@kirawi

kirawi commented Nov 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Should be good to review now. I'll add documentation if the changes look good. But to summarize:

backup.kind:
- None: never use a backup
- Copy: always use a copy backup
- Auto: use a copy backup if a rename is not safe/possible

backup.directories:
- Equivalent of Neovim's bdir

backup.extension:
- The extension with which backups end in

@kirawi kirawi added the S-waiting-on-review Status: Awaiting review from a maintainer. label Nov 18, 2024
@kirawi
kirawi force-pushed the open branch 3 times, most recently from df7fcec to d65f965 Compare November 18, 2024 18:47
@kirawi kirawi added the E-testing-wanted Call for participation: Experimental features suitable for testing label Nov 18, 2024
@kirawi

This comment was marked as resolved.

Sign in to view
@kirawi
kirawi force-pushed the open branch 2 times, most recently from aa53e90 to d65a2fb Compare December 31, 2024 05:15
@TornaxO7 TornaxO7 mentioned this pull request Feb 28, 2025
Open
aster-void added a commit to aster-void/helix that referenced this pull request Mar 1, 2025
@aster-void
style: rename config variables to match ones in helix-editor#11374 fo…
3136adc 
…r smoothier config transition
aster-void added a commit to aster-void/helix that referenced this pull request Mar 1, 2025
@aster-void
style: rename config variables to match ones in helix-editor#11374 fo…
da6d182 
…r smoothier config transition
aster-void added a commit to aster-void/helix that referenced this pull request Mar 1, 2025
@aster-void
style: rename config variables to match ones in helix-editor#11374 fo…
8b0a422 
…r smoothier config transition
@kirawi

kirawi commented Mar 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Problems with the PR I've noticed while testing it:

  • I forgot to implement the fail-over to copy backup if move fails.
  • There was an issue I encountered with Windows creating backups for administrator-only files (e.g. system32 files). Need to look into this.

@David-Else David-Else mentioned this pull request Mar 8, 2025
Open
aster-void added a commit to aster-void/helix that referenced this pull request Mar 31, 2025
@aster-void
style: rename config variables to match ones in helix-editor#11374 fo…
6b3a36e 
…r smoothier config transition
@the-mikedavis the-mikedavis mentioned this pull request Jul 8, 2025
Closed
archseer
archseer reviewed Sep 15, 2025
View reviewed changes
Comment thread helix-stdx/src/faccess.rs
}

pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> {
copy_ownership(from, to)?;

@archseer archseer Sep 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this PR is stalled can we at least extract the ownership fixes into a separate PR?

@kirawi kirawi Sep 18, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure

@kirawi kirawi Oct 14, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll be able to work on this now. Would you like me to revive this PR in its entirety, or extract just the ownership fixes for now and then submit separate PRs for the rest of the functionality?

@kirawi kirawi mentioned this pull request Oct 18, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@archseer archseer archseer left review comments
@pascalkuthe pascalkuthe Awaiting requested review from pascalkuthe

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-testing-wanted Call for participation: Experimental features suitable for testing S-waiting-on-review Status: Awaiting review from a maintainer.

Projects

None yet

Milestone

next

Development

Successfully merging this pull request may close these issues.

Align backup system to Neovim's

2 participants

@kirawi @archseer