Prefer language server formatting to external formatters

[the-mikedavis]

Auto-formatting currently prefers external formatters to language servers:

helix/helix-view/src/document.rs

Lines 725 to 815 in 0975d9c

	pub fn format(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterError>>> {
	if let Some((fmt_cmd, fmt_args)) = self
	.language_config()
	.and_then(|c| c.formatter.as_ref())
	.and_then(|formatter| {
	Some((
	helix_stdx::env::which(&formatter.command).ok()?,
	&formatter.args,
	))
	})
	{
	use std::process::Stdio;
	let text = self.text().clone();
	let mut process = tokio::process::Command::new(&fmt_cmd);
	process
	.args(fmt_args)
	.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)?;
	to_writer(&mut stdin, (encoding::UTF_8, false), &text)
	.await
	.map_err(|_| FormatterError::BrokenStdin)?;
	}
	let output = process
	.wait_with_output()
	.await
	.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).to_string()
	);
	}
	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();
	// finds first language server that supports formatting and then formats
	let language_server = self
	.language_servers_with_feature(LanguageServerFeature::Format)
	.next()?;
	let offset_encoding = language_server.offset_encoding();
	let request = language_server.text_document_formatting(
	self.identifier(),
	lsp::FormattingOptions {
	tab_size: self.tab_width() as u32,
	insert_spaces: matches!(self.indent_style, IndentStyle::Spaces(_)),
	..Default::default()
	},
	None,
	)?;
	let fut = async move {
	let edits = request.await.unwrap_or_else(|e| {
	log::warn!("LSP formatting failed: {}", e);
	Default::default()
	});
	Ok(helix_lsp::util::generate_transaction_from_edits(
	&text,
	edits,
	offset_encoding,
	))
	};
	Some(fut.boxed())
	}

But I believe this should be reversed: the language server is probably more efficient than us shelling out to the formatter and computing the diff ourselves.

If one wants to prefer the formatter to the language server formatting they can turn off the formatting feature for that language server using only-features / except-features (https://docs.helix-editor.com/languages.html#configuring-language-servers-for-a-language).

[David-Else]

The assumption at the moment is if you have installed an external formatter it is because you want to use it rather than the language server. That does make sense, and therefor this is a big change.

If you make it the default to prefer the language server formatter regardless, then it would also make sense to load up the defaults with all the best external formatters too, then people could easily choose between them by adding the except-features to choose the external. For example, as a default, adding Prettier as an external formatter for all the languages it can handle.

[janos-r]

In languages.toml, if I specify a formatter, than I obviously expect it to apply immediately, and not only after also looking up and specifying the default LSP and disabling it's formatter.