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.

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

fix: write-all crash #4384

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2515,19 +2515,20 @@ fn insert_at_line_end(cx: &mut Context) {
async fn make_format_callback(
doc_id: DocumentId,
doc_version: i32,
view_id: ViewId,
format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static,
write: Option<(Option<PathBuf>, bool)>,
) -> anyhow::Result<job::Callback> {
let format = format.await;

let call: job::Callback = Callback::Editor(Box::new(move |editor| {
if !editor.documents.contains_key(&doc_id) {
if !editor.documents.contains_key(&doc_id) || !editor.tree.contains(view_id) {
return;
}

let scrolloff = editor.config().scrolloff;
let doc = doc_mut!(editor, &doc_id);
let view = view_mut!(editor);
let view = view_mut!(editor, view_id);

if let Ok(format) = format {
if doc.version() == doc_version {
Expand Down
35 changes: 29 additions & 6 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,15 @@ fn write_impl(
) -> anyhow::Result<()> {
let editor_auto_fmt = cx.editor.config().auto_format;
let jobs = &mut cx.jobs;
let doc = doc_mut!(cx.editor);
let (view, doc) = current!(cx.editor);
let path = path.map(AsRef::as_ref);

let fmt = if editor_auto_fmt {
doc.auto_format().map(|fmt| {
let callback = make_format_callback(
doc.id(),
doc.version(),
view.id,
fmt,
Some((path.map(Into::into), force)),
);
Expand Down Expand Up @@ -343,9 +344,9 @@ fn format(
return Ok(());
}

let doc = doc!(cx.editor);
let (view, doc) = current!(cx.editor);
if let Some(format) = doc.format() {
let callback = make_format_callback(doc.id(), doc.version(), format, None);
let callback = make_format_callback(doc.id(), doc.version(), view.id, format, None);
cx.jobs.callback(callback);
}

Expand Down Expand Up @@ -573,12 +574,13 @@ fn write_all_impl(
let mut errors: Vec<&'static str> = Vec::new();
let auto_format = cx.editor.config().auto_format;
let jobs = &mut cx.jobs;
let current_view = view!(cx.editor);

// save all documents
let saves: Vec<_> = cx
.editor
.documents
.values()
.values_mut()
.filter_map(|doc| {
if doc.path().is_none() {
errors.push("cannot write a buffer without a filename\n");
Expand All @@ -589,10 +591,30 @@ fn write_all_impl(
return None;
}

// Look for a view to apply the formatting change to. If the document
// is in the current view, just use that. Otherwise, since we don't
// have any other metric available for better selection, just pick
// the first view arbitrarily so that we still commit the document
// state for undos. If somehow we have a document that has not been
// initialized with any view, initialize it with the current view.
let target_view = if doc.selections().contains_key(&current_view.id) {
current_view.id
} else if let Some(view) = doc.selections().keys().next() {
*view
} else {
doc.ensure_view_init(current_view.id);
current_view.id
};

let fmt = if auto_format {
doc.auto_format().map(|fmt| {
let callback =
make_format_callback(doc.id(), doc.version(), fmt, Some((None, force)));
let callback = make_format_callback(
doc.id(),
doc.version(),
target_view,
fmt,
Some((None, force)),
);
jobs.add(Job::with_callback(callback).wait_before_exiting());
})
} else {
Expand All @@ -602,6 +624,7 @@ fn write_all_impl(
if fmt.is_none() {
return Some(doc.id());
}

None
})
.collect();
Expand Down