From 62f6bb3c3f70df1984cbd1822874737dd75cf371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matou=C5=A1=20Dzivjak?= Date: Sun, 24 Dec 2023 01:13:00 +0100 Subject: [PATCH] [3209] feat(lsp): range formatting Add basic range formatting capabilities when multiple selection are present. Related: https://github.com/helix-editor/helix/issues/3209#issue-1318916766 --- helix-core/src/syntax.rs | 2 ++ helix-lsp/src/client.rs | 7 +++++ helix-term/src/commands.rs | 55 +++++++++++++++++++------------------- 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 93f618c09..cdc7a9e8c 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -313,6 +313,7 @@ where #[serde(rename_all = "kebab-case")] pub enum LanguageServerFeature { Format, + FormatSelection, GotoDeclaration, GotoDefinition, GotoTypeDefinition, @@ -338,6 +339,7 @@ impl Display for LanguageServerFeature { use LanguageServerFeature::*; let feature = match self { Format => "format", + FormatSelection => "format-selection", GotoDeclaration => "goto-declaration", GotoDefinition => "goto-definition", GotoTypeDefinition => "goto-type-definition", diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 643aa9a26..7b7ac3aec 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -287,6 +287,10 @@ impl Client { capabilities.document_formatting_provider, Some(OneOf::Left(true) | OneOf::Right(_)) ), + LanguageServerFeature::FormatSelection => matches!( + capabilities.document_range_formatting_provider, + Some(OneOf::Left(true) | OneOf::Right(_)) + ), LanguageServerFeature::GotoDeclaration => matches!( capabilities.declaration_provider, Some( @@ -662,6 +666,9 @@ impl Client { dynamic_registration: Some(false), resolve_support: None, }), + range_formatting: Some(lsp::DocumentFormattingClientCapabilities { + dynamic_registration: Some(false), + }), ..Default::default() }), window: Some(lsp::WindowClientCapabilities { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 7e0bee92b..572285ed8 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -63,6 +63,7 @@ use crate::{ }; use crate::job::{self, Jobs}; +use futures_util::future::join_all; use std::{ cmp::Ordering, collections::{HashMap, HashSet}, @@ -427,7 +428,7 @@ impl MappableCommand { paste_primary_clipboard_before, "Paste primary clipboard before selections", indent, "Indent selection", unindent, "Unindent selection", - format_selections, "Format selection", + format_selections, "Format selections", join_selections, "Join lines inside selection", join_selections_space, "Join lines inside selection and select spaces", keep_selections, "Keep selections matching regex", @@ -4469,25 +4470,11 @@ fn format_selections(cx: &mut Context) { let (view, doc) = current!(cx.editor); let view_id = view.id; - // via lsp if available - // TODO: else via tree-sitter indentation calculations + // TODO: via lsp if available else via tree-sitter indentation calculations - if doc.selection(view_id).len() != 1 { - cx.editor - .set_error("format_selections only supports a single selection for now"); - return; - } - - // TODO extra LanguageServerFeature::FormatSelections? - // maybe such that LanguageServerFeature::Format contains it as well let Some(language_server) = doc - .language_servers_with_feature(LanguageServerFeature::Format) - .find(|ls| { - matches!( - ls.capabilities().document_range_formatting_provider, - Some(lsp::OneOf::Left(true) | lsp::OneOf::Right(_)) - ) - }) + .language_servers_with_feature(LanguageServerFeature::FormatSelection) + .next() else { cx.editor .set_error("No configured language server supports range formatting"); @@ -4498,16 +4485,15 @@ fn format_selections(cx: &mut Context) { let ranges: Vec = doc .selection(view_id) .iter() + // request and process range formatting in reverse order from last selection + // to the first selection to reduce the chances of collisions (change in earlier + // sections could cause offsets in later sections, can't happen the other way around). + .rev() .map(|range| range_to_lsp_range(doc.text(), *range, offset_encoding)) .collect(); - // TODO: handle fails - // TODO: concurrent map over all ranges - - let range = ranges[0]; - - let future = language_server - .text_document_range_formatting( + let futures = ranges.into_iter().filter_map(|range| { + language_server.text_document_range_formatting( doc.identifier(), range, lsp::FormattingOptions { @@ -4517,12 +4503,25 @@ fn format_selections(cx: &mut Context) { }, None, ) - .unwrap(); + }); + + let results = helix_lsp::block_on(join_all(futures)); - let edits = tokio::task::block_in_place(|| helix_lsp::block_on(future)).unwrap_or_default(); + let all_edits = results + .into_iter() + .filter_map(|result| { + match result { + // TODO: handle colliding edits (edits outside the range) and edits that result into collision. + // See: https://github.com/helix-editor/helix/issues/3209#issuecomment-1197463913 + Ok(edits) => Some(edits), + Err(_) => None, + } + }) + .flatten() + .collect(); let transaction = - helix_lsp::util::generate_transaction_from_edits(doc.text(), edits, offset_encoding); + helix_lsp::util::generate_transaction_from_edits(doc.text(), all_edits, offset_encoding); doc.apply(&transaction, view_id); }