mirror of https://github.com/helix-editor/helix
fix: write-all crash (#4384)
When we do auto formatting, the code that takes the LSP's response and applies the changes to the document are just getting the currently focused view and giving that to the function, basically always assuming that the document that we're applying the change to is in focus, and not in a background view. This is usually fine for a single view, even if it's a buffer in the background, because it's still the same view and the selection will get updated accordingly for when you switch back to it. But it's obviously a problem for when there are multiple views, because if you don't have the target document in focus, it will ask the document to update the wrong view, hence the crash. The problem with this is picking which view to apply any selection change to. In the absence of any more data points on the views themselves, we simply pick the first view associated with the document we are saving.pull/4406/head
parent
bad49ef2d0
commit
5a848344a9
|
@ -2515,19 +2515,20 @@ fn insert_at_line_end(cx: &mut Context) {
|
||||||
async fn make_format_callback(
|
async fn make_format_callback(
|
||||||
doc_id: DocumentId,
|
doc_id: DocumentId,
|
||||||
doc_version: i32,
|
doc_version: i32,
|
||||||
|
view_id: ViewId,
|
||||||
format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static,
|
format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static,
|
||||||
write: Option<(Option<PathBuf>, bool)>,
|
write: Option<(Option<PathBuf>, bool)>,
|
||||||
) -> anyhow::Result<job::Callback> {
|
) -> anyhow::Result<job::Callback> {
|
||||||
let format = format.await;
|
let format = format.await;
|
||||||
|
|
||||||
let call: job::Callback = Callback::Editor(Box::new(move |editor| {
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let scrolloff = editor.config().scrolloff;
|
let scrolloff = editor.config().scrolloff;
|
||||||
let doc = doc_mut!(editor, &doc_id);
|
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 let Ok(format) = format {
|
||||||
if doc.version() == doc_version {
|
if doc.version() == doc_version {
|
||||||
|
|
|
@ -271,7 +271,7 @@ fn write_impl(
|
||||||
) -> anyhow::Result<()> {
|
) -> anyhow::Result<()> {
|
||||||
let editor_auto_fmt = cx.editor.config().auto_format;
|
let editor_auto_fmt = cx.editor.config().auto_format;
|
||||||
let jobs = &mut cx.jobs;
|
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 path = path.map(AsRef::as_ref);
|
||||||
|
|
||||||
let fmt = if editor_auto_fmt {
|
let fmt = if editor_auto_fmt {
|
||||||
|
@ -279,6 +279,7 @@ fn write_impl(
|
||||||
let callback = make_format_callback(
|
let callback = make_format_callback(
|
||||||
doc.id(),
|
doc.id(),
|
||||||
doc.version(),
|
doc.version(),
|
||||||
|
view.id,
|
||||||
fmt,
|
fmt,
|
||||||
Some((path.map(Into::into), force)),
|
Some((path.map(Into::into), force)),
|
||||||
);
|
);
|
||||||
|
@ -344,9 +345,9 @@ fn format(
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
let doc = doc!(cx.editor);
|
let (view, doc) = current!(cx.editor);
|
||||||
if let Some(format) = doc.format() {
|
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);
|
cx.jobs.callback(callback);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -568,12 +569,13 @@ pub fn write_all_impl(
|
||||||
let mut errors: Vec<&'static str> = Vec::new();
|
let mut errors: Vec<&'static str> = Vec::new();
|
||||||
let auto_format = cx.editor.config().auto_format;
|
let auto_format = cx.editor.config().auto_format;
|
||||||
let jobs = &mut cx.jobs;
|
let jobs = &mut cx.jobs;
|
||||||
|
let current_view = view!(cx.editor);
|
||||||
|
|
||||||
// save all documents
|
// save all documents
|
||||||
let saves: Vec<_> = cx
|
let saves: Vec<_> = cx
|
||||||
.editor
|
.editor
|
||||||
.documents
|
.documents
|
||||||
.values()
|
.values_mut()
|
||||||
.filter_map(|doc| {
|
.filter_map(|doc| {
|
||||||
if !doc.is_modified() {
|
if !doc.is_modified() {
|
||||||
return None;
|
return None;
|
||||||
|
@ -585,10 +587,30 @@ pub fn write_all_impl(
|
||||||
return None;
|
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(¤t_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 {
|
let fmt = if auto_format {
|
||||||
doc.auto_format().map(|fmt| {
|
doc.auto_format().map(|fmt| {
|
||||||
let callback =
|
let callback = make_format_callback(
|
||||||
make_format_callback(doc.id(), doc.version(), fmt, Some((None, force)));
|
doc.id(),
|
||||||
|
doc.version(),
|
||||||
|
target_view,
|
||||||
|
fmt,
|
||||||
|
Some((None, force)),
|
||||||
|
);
|
||||||
jobs.add(Job::with_callback(callback).wait_before_exiting());
|
jobs.add(Job::with_callback(callback).wait_before_exiting());
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
|
@ -598,6 +620,7 @@ pub fn write_all_impl(
|
||||||
if fmt.is_none() {
|
if fmt.is_none() {
|
||||||
return Some(doc.id());
|
return Some(doc.id());
|
||||||
}
|
}
|
||||||
|
|
||||||
None
|
None
|
||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|
Loading…
Reference in New Issue