From 9d5e9326266d6b064c6d0f5d78ba2fae4d78e8fb Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Mon, 21 Oct 2019 02:04:55 +0300 Subject: fixup folding ranges for "lineFoldingOnly" clients #2033 --- crates/ra_lsp_server/src/conv.rs | 63 ++++++++++++++++++++------ crates/ra_lsp_server/src/main_loop.rs | 24 ++++++---- crates/ra_lsp_server/src/main_loop/handlers.rs | 10 +++- crates/ra_lsp_server/src/world.rs | 1 + 4 files changed, 74 insertions(+), 24 deletions(-) (limited to 'crates') diff --git a/crates/ra_lsp_server/src/conv.rs b/crates/ra_lsp_server/src/conv.rs index 1318a1738..173580dee 100644 --- a/crates/ra_lsp_server/src/conv.rs +++ b/crates/ra_lsp_server/src/conv.rs @@ -227,22 +227,57 @@ impl ConvWith<(&LineIndex, LineEndings)> for &AtomTextEdit { } } -impl ConvWith<&LineIndex> for Fold { +pub(crate) struct FoldConvCtx<'a> { + pub(crate) text: &'a str, + pub(crate) line_index: &'a LineIndex, + pub(crate) line_folding_only: bool, +} + +impl ConvWith<&FoldConvCtx<'_>> for Fold { type Output = lsp_types::FoldingRange; - fn conv_with(self, line_index: &LineIndex) -> lsp_types::FoldingRange { - let range = self.range.conv_with(&line_index); - lsp_types::FoldingRange { - start_line: range.start.line, - start_character: Some(range.start.character), - end_line: range.end.line, - end_character: Some(range.end.character), - kind: match self.kind { - FoldKind::Comment => Some(lsp_types::FoldingRangeKind::Comment), - FoldKind::Imports => Some(lsp_types::FoldingRangeKind::Imports), - FoldKind::Mods => None, - FoldKind::Block => None, - }, + fn conv_with(self, ctx: &FoldConvCtx) -> lsp_types::FoldingRange { + let kind = match self.kind { + FoldKind::Comment => Some(lsp_types::FoldingRangeKind::Comment), + FoldKind::Imports => Some(lsp_types::FoldingRangeKind::Imports), + FoldKind::Mods => None, + FoldKind::Block => None, + }; + + let range = self.range.conv_with(&ctx.line_index); + + if ctx.line_folding_only { + // Clients with line_folding_only == true (such as VSCode) will fold the whole end line + // even if it contains text not in the folding range. To prevent that we exclude + // range.end.line from the folding region if there is more text after range.end + // on the same line. + let has_more_text_on_end_line = ctx.text + [TextRange::from_to(self.range.end(), TextUnit::of_str(ctx.text))] + .chars() + .take_while(|it| *it != '\n') + .any(|it| !it.is_whitespace()); + + let end_line = if has_more_text_on_end_line { + range.end.line.saturating_sub(1) + } else { + range.end.line + }; + + lsp_types::FoldingRange { + start_line: range.start.line, + start_character: None, + end_line, + end_character: None, + kind, + } + } else { + lsp_types::FoldingRange { + start_line: range.start.line, + start_character: Some(range.start.character), + end_line: range.end.line, + end_character: Some(range.end.character), + kind, + } } } } diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 35c35d32b..0b5d9c44d 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -111,6 +111,21 @@ pub fn main_loop( connection.sender.send(request.into()).unwrap(); } + let options = { + let text_document_caps = client_caps.text_document.as_ref(); + Options { + publish_decorations: config.publish_decorations, + supports_location_link: text_document_caps + .and_then(|it| it.definition) + .and_then(|it| it.link_support) + .unwrap_or(false), + line_folding_only: text_document_caps + .and_then(|it| it.folding_range.as_ref()) + .and_then(|it| it.line_folding_only) + .unwrap_or(false), + } + }; + let feature_flags = { let mut ff = FeatureFlags::default(); for (flag, value) in config.feature_flags { @@ -133,14 +148,7 @@ pub fn main_loop( config.lru_capacity, &globs, Watch(!config.use_client_watching), - Options { - publish_decorations: config.publish_decorations, - supports_location_link: client_caps - .text_document - .and_then(|it| it.definition) - .and_then(|it| it.link_support) - .unwrap_or(false), - }, + options, feature_flags, ) }; diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 10e271376..af3cd04ea 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -18,7 +18,7 @@ use serde_json::to_value; use crate::{ cargo_target_spec::{runnable_args, CargoTargetSpec}, - conv::{to_location, Conv, ConvWith, MapConvWith, TryConvWith, TryConvWithToVec}, + conv::{to_location, Conv, ConvWith, FoldConvCtx, MapConvWith, TryConvWith, TryConvWithToVec}, req::{self, Decoration, InlayHint, InlayHintsParams, InlayKind}, world::WorldSnapshot, LspError, Result, @@ -383,8 +383,14 @@ pub fn handle_folding_range( ) -> Result>> { let file_id = params.text_document.try_conv_with(&world)?; let folds = world.analysis().folding_ranges(file_id)?; + let text = world.analysis().file_text(file_id)?; let line_index = world.analysis().file_line_index(file_id)?; - let res = Some(folds.into_iter().map_conv_with(&*line_index).collect()); + let ctx = FoldConvCtx { + text: &text, + line_index: &line_index, + line_folding_only: world.options.line_folding_only, + }; + let res = Some(folds.into_iter().map_conv_with(&ctx).collect()); Ok(res) } diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 0eb684de5..51824e7a3 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -27,6 +27,7 @@ use crate::{ pub struct Options { pub publish_decorations: bool, pub supports_location_link: bool, + pub line_folding_only: bool, } /// `WorldState` is the primary mutable state of the language server -- cgit v1.2.3 From 6d105ccd93b8793592a6e89872766fcaf6c822e4 Mon Sep 17 00:00:00 2001 From: Alex Zatelepin Date: Mon, 21 Oct 2019 22:34:44 +0300 Subject: add test #2033 --- crates/ra_lsp_server/src/conv.rs | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'crates') diff --git a/crates/ra_lsp_server/src/conv.rs b/crates/ra_lsp_server/src/conv.rs index 173580dee..ee503633d 100644 --- a/crates/ra_lsp_server/src/conv.rs +++ b/crates/ra_lsp_server/src/conv.rs @@ -547,3 +547,46 @@ where self.map(|it| it.try_conv_with(ctx)).collect() } } + +#[cfg(test)] +mod tests { + use super::*; + use test_utils::extract_ranges; + + #[test] + fn conv_fold_line_folding_only_fixup() { + let text = r#"mod a; +mod b; +mod c; + +fn main() { + if cond { + a::do_a(); + } else { + b::do_b(); + } +}"#; + + let (ranges, text) = extract_ranges(text, "fold"); + assert_eq!(ranges.len(), 4); + let folds = vec![ + Fold { range: ranges[0], kind: FoldKind::Mods }, + Fold { range: ranges[1], kind: FoldKind::Block }, + Fold { range: ranges[2], kind: FoldKind::Block }, + Fold { range: ranges[3], kind: FoldKind::Block }, + ]; + + let line_index = LineIndex::new(&text); + let ctx = FoldConvCtx { text: &text, line_index: &line_index, line_folding_only: true }; + let converted: Vec<_> = folds.into_iter().map_conv_with(&ctx).collect(); + + let expected_lines = [(0, 2), (4, 10), (5, 6), (7, 9)]; + assert_eq!(converted.len(), expected_lines.len()); + for (folding_range, (start_line, end_line)) in converted.iter().zip(expected_lines.iter()) { + assert_eq!(folding_range.start_line, *start_line); + assert_eq!(folding_range.start_character, None); + assert_eq!(folding_range.end_line, *end_line); + assert_eq!(folding_range.end_character, None); + } + } +} -- cgit v1.2.3