Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: choose backup strategy #12985

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat: choose backup strategy #12985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
66756e0
feat: choose writing strategy
aster-void Feb 27, 2025
41ed683
style: rename writing_strategy -> backup_strategy
aster-void Feb 28, 2025
8b0a422
style: rename config variables to match ones in #11374 for smoothier …
aster-void Mar 1, 2025
aff08e0
fix: delete backup after successfully restoring backup in copy strategy
aster-void Mar 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 119 additions & 63 deletions helix-view/src/document.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,26 @@ pub enum Mode {
Insert = 2,
}

#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct BackupConfig {
pub kind: BackupConfigKind,
}
#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum BackupConfigKind {
#[default]
Auto,
Copy,
None,
}
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum BackupKind {
Move,
Copy,
None,
}

impl Display for Mode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Expand Down Expand Up @@ -918,6 +938,7 @@ impl Document {
let encoding_with_bom_info = (self.encoding, self.has_bom);
let last_saved_time = self.last_saved_time;

let backup_config_kind = self.config.load().backup.kind;
// We encode the file according to the `Document`'s encoding.
let future = async move {
use tokio::fs;
Expand Down Expand Up @@ -961,38 +982,58 @@ impl Document {
));
}

// Assume it is a hardlink to prevent data loss if the metadata cant be read (e.g. on certain Windows configurations)
let is_hardlink = helix_stdx::faccess::hardlink_count(&write_path).unwrap_or(2) > 1;
let backup = if path.exists() {
let path_ = write_path.clone();
// hacks: we use tempfile to handle the complex task of creating
// non clobbered temporary path for us we don't want
// the whole automatically delete path on drop thing
// since the path doesn't exist yet, we just want
// the path
tokio::task::spawn_blocking(move || -> Option<PathBuf> {
let mut builder = tempfile::Builder::new();
builder.prefix(path_.file_name()?).suffix(".bck");

let backup_path = if is_hardlink {
builder
.make_in(path_.parent()?, |backup| std::fs::copy(&path_, backup))
.ok()?
.into_temp_path()
} else {
builder
.make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup))
let backup_kind = {
match backup_config_kind {
BackupConfigKind::Copy => BackupKind::Copy,
BackupConfigKind::None => BackupKind::None,
BackupConfigKind::Auto => {
// Assume it is a hardlink to prevent data loss if the metadata cant be read (e.g. on certain Windows configurations)
let is_hardlink =
helix_stdx::faccess::hardlink_count(&write_path).unwrap_or(2) > 1;

if is_hardlink {
BackupKind::Copy
} else {
BackupKind::Move
}
}
}
};

let backup = match backup_kind {
BackupKind::Copy | BackupKind::Move if path.exists() => {
let path_ = write_path.clone();
// hacks: we use tempfile to handle the complex task of creating
// non clobbered temporary path for us we don't want
// the whole automatically delete path on drop thing
// since the path doesn't exist yet, we just want
// the path
tokio::task::spawn_blocking(move || -> Option<PathBuf> {
let backup_path = tempfile::Builder::new()
.prefix(path_.file_name()?)
.suffix(".bck")
.make_in(path_.parent()?, |backup| {
match backup_kind {
BackupKind::Copy => {
std::fs::copy(&path_, backup)?;
}
BackupKind::Move => {
std::fs::rename(&path_, backup)?;
}
BackupKind::None => unreachable!(),
}
Ok(())
})
.ok()?
.into_temp_path()
};
.into_temp_path();

backup_path.keep().ok()
})
.await
.ok()
.flatten()
} else {
None
backup_path.keep().ok()
})
.await
.ok()
.flatten()
}
_ => None,
};

let write_result: anyhow::Result<_> = async {
Expand All @@ -1003,47 +1044,62 @@ impl Document {
}
.await;

let save_time = match fs::metadata(&write_path).await {
Ok(metadata) => metadata.modified().map_or(SystemTime::now(), |mtime| mtime),
Err(_) => SystemTime::now(),
};

if let Some(backup) = backup {
if is_hardlink {
let mut delete = true;
if write_result.is_err() {
// Restore backup
let _ = tokio::fs::copy(&backup, &write_path).await.map_err(|e| {
delete = false;
log::error!("Failed to restore backup on write failure: {e}")
});
}

if delete {
// Delete backup
let _ = tokio::fs::remove_file(backup)
.await
.map_err(|e| log::error!("Failed to remove backup file on write: {e}"));
let save_time = fs::metadata(&write_path)
.await
.and_then(|metadata| metadata.modified())
.unwrap_or_else(|_| SystemTime::now());

if let Err(write_err) = write_result {
if let Some(backup) = backup {
match backup_kind {
BackupKind::Copy => {
// Restore backup
if let Err(restore_err) = tokio::fs::copy(&backup, &write_path).await {
log::error!(
"Failed to restore backup on write failure: {restore_err}"
)
} else {
// successfully restored. delete backup
if let Err(delete_err) = tokio::fs::remove_file(backup).await {
log::error!(
"Failed to remove backup file on write: {delete_err}"
);
}
}
}
BackupKind::Move => {
// restore backup
if let Err(restore_err) = tokio::fs::rename(&backup, &write_path).await
{
log::error!(
"Failed to restore backup on write failure: {restore_err}"
);
}
}
BackupKind::None => unreachable!(),
}
} else if write_result.is_err() {
// restore backup
let _ = tokio::fs::rename(&backup, &write_path)
.await
.map_err(|e| log::error!("Failed to restore backup on write failure: {e}"));
} else {
// copy metadata and delete backup
log::error!(
"Failed to restore backup on write failure (backup doesn't exist) for write error: {write_err}"
);
}
} else if let Some(backup) = backup {
// backup exists & successfully saved. delete backup
if backup_kind == BackupKind::Move {
// the file is newly created one, therefore the metadata must be copied
let backup = backup.clone();
let _ = tokio::task::spawn_blocking(move || {
let _ = copy_metadata(&backup, &write_path)
.map_err(|e| log::error!("Failed to copy metadata on write: {e}"));
let _ = std::fs::remove_file(backup)
.map_err(|e| log::error!("Failed to remove backup file on write: {e}"));
if let Err(e) = copy_metadata(&backup, &write_path) {
log::error!("Failed to copy metadata on write: {e}");
}
})
.await;
}
if let Err(e) = tokio::fs::remove_file(backup).await {
log::error!("Failed to remove backup file on write: {e}");
}
}

write_result?;

let event = DocumentSavedEvent {
revision: current_rev,
save_time,
Expand Down
5 changes: 4 additions & 1 deletion helix-view/src/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use crate::{
annotations::diagnostics::{DiagnosticFilter, InlineDiagnosticsConfig},
clipboard::ClipboardProvider,
document::{
DocumentOpenError, DocumentSavedEventFuture, DocumentSavedEventResult, Mode, SavePoint,
BackupConfig, DocumentOpenError, DocumentSavedEventFuture, DocumentSavedEventResult, Mode,
SavePoint,
},
events::DocumentFocusLost,
graphics::{CursorKind, Rect},
Expand Down Expand Up @@ -360,6 +361,7 @@ pub struct Config {
pub end_of_line_diagnostics: DiagnosticFilter,
// Set to override the default clipboard provider
pub clipboard_provider: ClipboardProvider,
pub backup: BackupConfig,
}

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)]
Expand Down Expand Up @@ -1001,6 +1003,7 @@ impl Default for Config {
inline_diagnostics: InlineDiagnosticsConfig::default(),
end_of_line_diagnostics: DiagnosticFilter::Disable,
clipboard_provider: ClipboardProvider::default(),
backup: BackupConfig::default(),
}
}
}
Expand Down