minimize diff

A few edits here to minimize the overall (squashed) diff:

* Minimize borrow-checker-satisfying changes in write_impl,
  write_all_impl. We just need to "refresh" the bindings of `doc` and
  `view` It's nice to avoid extra blocks for the `&mut` bindings since
  they unnecessarily add indentation/nesting.
* Factor out the `Result<Vec<_>, _>` early in `Document::format`: this
  avoids changing the indentation for the rest of the block.

With these changes the stat (git diff --stat 9f3b193743) lowers from
+128,-82 to +51,-10.

Also there's a style change here to prefer `use`-ing the
`crate::expansion` module so it's clearer where the `expand` function
comes from.
pull/13429/head
Michael Davis 2025-04-28 18:07:39 -04:00
parent 3763fc3c54
commit 3d8a63e072
No known key found for this signature in database
2 changed files with 87 additions and 92 deletions

View File

@ -324,22 +324,20 @@ fn buffer_previous(
fn write_impl(cx: &mut compositor::Context, path: Option<&str>, force: bool) -> anyhow::Result<()> { fn write_impl(cx: &mut compositor::Context, path: Option<&str>, force: bool) -> anyhow::Result<()> {
let config = cx.editor.config(); let config = cx.editor.config();
let jobs = &mut cx.jobs; let jobs = &mut cx.jobs;
{ let (view, doc) = current!(cx.editor);
let (view, doc) = current!(cx.editor);
if doc.trim_trailing_whitespace() { if doc.trim_trailing_whitespace() {
trim_trailing_whitespace(doc, view.id); trim_trailing_whitespace(doc, view.id);
}
if config.trim_final_newlines {
trim_final_newlines(doc, view.id);
}
if doc.insert_final_newline() {
insert_final_newline(doc, view.id);
}
// Save an undo checkpoint for any outstanding changes.
doc.append_changes_to_history(view);
} }
if config.trim_final_newlines {
trim_final_newlines(doc, view.id);
}
if doc.insert_final_newline() {
insert_final_newline(doc, view.id);
}
// Save an undo checkpoint for any outstanding changes.
doc.append_changes_to_history(view);
let (view, doc) = current_ref!(cx.editor); let (view, doc) = current_ref!(cx.editor);
let fmt = if config.auto_format { let fmt = if config.auto_format {
@ -738,26 +736,24 @@ pub fn write_all_impl(
.collect(); .collect();
for (doc_id, target_view) in saves { for (doc_id, target_view) in saves {
{ let doc = doc_mut!(cx.editor, &doc_id);
let doc = doc_mut!(cx.editor, &doc_id); let view = view_mut!(cx.editor, target_view);
let view = view_mut!(cx.editor, target_view);
if doc.trim_trailing_whitespace() { if doc.trim_trailing_whitespace() {
trim_trailing_whitespace(doc, target_view); trim_trailing_whitespace(doc, target_view);
} }
if config.trim_final_newlines { if config.trim_final_newlines {
trim_final_newlines(doc, target_view); trim_final_newlines(doc, target_view);
} }
if doc.insert_final_newline() { if doc.insert_final_newline() {
insert_final_newline(doc, target_view); insert_final_newline(doc, target_view);
}
// Save an undo checkpoint for any outstanding changes.
doc.append_changes_to_history(view);
} }
let doc = doc!(cx.editor, &doc_id); // Save an undo checkpoint for any outstanding changes.
doc.append_changes_to_history(view);
let fmt = if options.auto_format && config.auto_format { let fmt = if options.auto_format && config.auto_format {
let doc = doc!(cx.editor, &doc_id);
doc.auto_format(cx.editor).map(|fmt| { doc.auto_format(cx.editor).map(|fmt| {
let callback = make_format_callback( let callback = make_format_callback(
doc_id, doc_id,

View File

@ -5,7 +5,6 @@ use futures_util::future::BoxFuture;
use futures_util::FutureExt; use futures_util::FutureExt;
use helix_core::auto_pairs::AutoPairs; use helix_core::auto_pairs::AutoPairs;
use helix_core::chars::char_is_word; use helix_core::chars::char_is_word;
use helix_core::command_line::Token; use helix_core::command_line::Token;
use helix_core::diagnostic::DiagnosticProvider; use helix_core::diagnostic::DiagnosticProvider;
use helix_core::doc_formatter::TextFormat; use helix_core::doc_formatter::TextFormat;
@ -44,10 +43,10 @@ use helix_core::{
ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction, ChangeSet, Diagnostic, LineEnding, Range, Rope, RopeBuilder, Selection, Syntax, Transaction,
}; };
use crate::expansion::expand;
use crate::{ use crate::{
editor::Config, editor::Config,
events::{DocumentDidChange, SelectionDidChange}, events::{DocumentDidChange, SelectionDidChange},
expansion,
view::ViewPosition, view::ViewPosition,
DocumentId, Editor, Theme, View, ViewId, DocumentId, Editor, Theme, View, ViewId,
}; };
@ -823,68 +822,68 @@ impl Document {
process.current_dir(doc_dir); process.current_dir(doc_dir);
} }
let fmt_args = fmt_args let args = match fmt_args
.iter() .iter()
.map(|content| expand(editor, Token::expand(content))) .map(|content| expansion::expand(editor, Token::expand(content)))
.collect::<anyhow::Result<Vec<_>>>(); .collect::<Result<Vec<_>, _>>()
{
match fmt_args { Ok(args) => args,
Ok(fmt_args) => { Err(err) => {
process log::error!("Failed to expand formatter arguments: {err}");
.args(fmt_args.iter().map(Cow::as_ref)) return None;
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped());
let formatting_future = async move {
let mut process =
process
.spawn()
.map_err(|e| FormatterError::SpawningFailed {
command: fmt_cmd.to_string_lossy().into(),
error: e.kind(),
})?;
let mut stdin = process.stdin.take().ok_or(FormatterError::BrokenStdin)?;
let input_text = text.clone();
let input_task = tokio::spawn(async move {
to_writer(&mut stdin, (encoding::UTF_8, false), &input_text).await
// Note that `stdin` is dropped here, causing the pipe to close. This can
// avoid a deadlock with `wait_with_output` below if the process is waiting on
// stdin to close before exiting.
});
let (input_result, output_result) = tokio::join! {
input_task,
process.wait_with_output(),
};
let _ = input_result.map_err(|_| FormatterError::BrokenStdin)?;
let output =
output_result.map_err(|_| FormatterError::WaitForOutputFailed)?;
if !output.status.success() {
if !output.stderr.is_empty() {
let err = String::from_utf8_lossy(&output.stderr).to_string();
log::error!("Formatter error: {}", err);
return Err(FormatterError::NonZeroExitStatus(Some(err)));
}
return Err(FormatterError::NonZeroExitStatus(None));
} else if !output.stderr.is_empty() {
log::debug!(
"Formatter printed to stderr: {}",
String::from_utf8_lossy(&output.stderr)
);
}
let str = std::str::from_utf8(&output.stdout)
.map_err(|_| FormatterError::InvalidUtf8Output)?;
Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str)))
};
return Some(formatting_future.boxed());
} }
Err(e) => log::error!("{e}"), };
}
process
.args(args.iter().map(AsRef::as_ref))
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped());
let formatting_future = async move {
let mut process = process
.spawn()
.map_err(|e| FormatterError::SpawningFailed {
command: fmt_cmd.to_string_lossy().into(),
error: e.kind(),
})?;
let mut stdin = process.stdin.take().ok_or(FormatterError::BrokenStdin)?;
let input_text = text.clone();
let input_task = tokio::spawn(async move {
to_writer(&mut stdin, (encoding::UTF_8, false), &input_text).await
// Note that `stdin` is dropped here, causing the pipe to close. This can
// avoid a deadlock with `wait_with_output` below if the process is waiting on
// stdin to close before exiting.
});
let (input_result, output_result) = tokio::join! {
input_task,
process.wait_with_output(),
};
let _ = input_result.map_err(|_| FormatterError::BrokenStdin)?;
let output = output_result.map_err(|_| FormatterError::WaitForOutputFailed)?;
if !output.status.success() {
if !output.stderr.is_empty() {
let err = String::from_utf8_lossy(&output.stderr).to_string();
log::error!("Formatter error: {}", err);
return Err(FormatterError::NonZeroExitStatus(Some(err)));
}
return Err(FormatterError::NonZeroExitStatus(None));
} else if !output.stderr.is_empty() {
log::debug!(
"Formatter printed to stderr: {}",
String::from_utf8_lossy(&output.stderr)
);
}
let str = std::str::from_utf8(&output.stdout)
.map_err(|_| FormatterError::InvalidUtf8Output)?;
Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str)))
};
return Some(formatting_future.boxed());
}; };
let text = self.text.clone(); let text = self.text.clone();