Skip to content

Commit

Permalink
Fix handling of selection ranges for format selections in multibuffer (…
Browse files Browse the repository at this point in the history 
…#22929)

Before this change it was using the same multibuffer point ranges in
every buffer, which only worked correctly for singleton buffers.

Release Notes:

- Fixed handling of selection ranges when formatting selections within a
multibuffer.

---------

Co-authored-by: Thorsten Ball <[email protected]>
  • Loading branch information
@mgsloan @mrnugget
mgsloan and mrnugget authored Jan 10, 2025
1 parent 29aa291 commit 685dd77
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 119 deletions.
14 changes: 7 additions & 7 deletions crates/collab/src/tests/integration_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ use language::{
};
use lsp::LanguageServerId;
use parking_lot::Mutex;
use project::lsp_store::FormatTarget;
use project::{
lsp_store::FormatTrigger, search::SearchQuery, search::SearchResult, DiagnosticSummary,
HoverBlockKind, Project, ProjectPath,
lsp_store::{FormatTrigger, LspFormatTarget},
search::{SearchQuery, SearchResult},
DiagnosticSummary, HoverBlockKind, Project, ProjectPath,
};
use rand::prelude::*;
use serde_json::json;
Expand Down Expand Up @@ -4400,9 +4400,9 @@ async fn test_formatting_buffer(
.update(cx_b, |project, cx| {
project.format(
HashSet::from_iter([buffer_b.clone()]),
LspFormatTarget::Buffers,
true,
FormatTrigger::Save,
FormatTarget::Buffer,
cx,
)
})
Expand Down Expand Up @@ -4436,9 +4436,9 @@ async fn test_formatting_buffer(
.update(cx_b, |project, cx| {
project.format(
HashSet::from_iter([buffer_b.clone()]),
LspFormatTarget::Buffers,
true,
FormatTrigger::Save,
FormatTarget::Buffer,
cx,
)
})
Expand Down Expand Up @@ -4546,9 +4546,9 @@ async fn test_prettier_formatting_buffer(
.update(cx_b, |project, cx| {
project.format(
HashSet::from_iter([buffer_b.clone()]),
LspFormatTarget::Buffers,
true,
FormatTrigger::Save,
FormatTarget::Buffer,
cx,
)
})
Expand All @@ -4566,9 +4566,9 @@ async fn test_prettier_formatting_buffer(
.update(cx_a, |project, cx| {
project.format(
HashSet::from_iter([buffer_a.clone()]),
LspFormatTarget::Buffers,
true,
FormatTrigger::Manual,
FormatTarget::Buffer,
cx,
)
})
Expand Down
6 changes: 3 additions & 3 deletions crates/collab/src/tests/remote_editing_collaboration_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use language::{
};
use node_runtime::NodeRuntime;
use project::{
lsp_store::{FormatTarget, FormatTrigger},
lsp_store::{FormatTrigger, LspFormatTarget},
ProjectPath,
};
use remote::SshRemoteClient;
Expand Down Expand Up @@ -472,9 +472,9 @@ async fn test_ssh_collaboration_formatting_with_prettier(
.update(cx_b, |project, cx| {
project.format(
HashSet::from_iter([buffer_b.clone()]),
LspFormatTarget::Buffers,
true,
FormatTrigger::Save,
FormatTarget::Buffer,
cx,
)
})
Expand Down Expand Up @@ -509,9 +509,9 @@ async fn test_ssh_collaboration_formatting_with_prettier(
.update(cx_a, |project, cx| {
project.format(
HashSet::from_iter([buffer_a.clone()]),
LspFormatTarget::Buffers,
true,
FormatTrigger::Manual,
FormatTarget::Buffer,
cx,
)
})
Expand Down
52 changes: 42 additions & 10 deletions crates/editor/src/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ use multi_buffer::{
};
use project::{
buffer_store::BufferChangeSet,
lsp_store::{FormatTarget, FormatTrigger, OpenLspBufferHandle},
lsp_store::{FormatTrigger, LspFormatTarget, OpenLspBufferHandle},
project_settings::{GitGutterSetting, ProjectSettings},
CodeAction, Completion, CompletionIntent, DocumentHighlight, InlayHint, Location, LocationLink,
LspStore, Project, ProjectItem, ProjectTransaction, TaskSourceKind,
Expand Down Expand Up @@ -986,6 +986,11 @@ impl InlayHintRefreshReason {
}
}

pub enum FormatTarget {
Buffers,
Ranges(Vec<Range<MultiBufferPoint>>),
}

pub(crate) struct FocusedBlock {
id: BlockId,
focus_handle: WeakFocusHandle,
Expand Down Expand Up @@ -10237,7 +10242,7 @@ impl Editor {
None => return None,
};

Some(self.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffer, cx))
Some(self.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffers, cx))
}

fn format_selections(
Expand All @@ -10250,17 +10255,18 @@ impl Editor {
None => return None,
};

let selections = self
let ranges = self
.selections
.all_adjusted(cx)
.into_iter()
.map(|selection| selection.range())
.filter(|s| !s.is_empty())
.collect_vec();

Some(self.perform_format(
project,
FormatTrigger::Manual,
FormatTarget::Ranges(selections),
FormatTarget::Ranges(ranges),
cx,
))
}
Expand All @@ -10272,15 +10278,41 @@ impl Editor {
target: FormatTarget,
cx: &mut ViewContext<Self>,
) -> Task<Result<()>> {
let buffer = self.buffer().clone();
let mut buffers = buffer.read(cx).all_buffers();
if trigger == FormatTrigger::Save {
buffers.retain(|buffer| buffer.read(cx).is_dirty());
}
let buffer = self.buffer.clone();
let (buffers, target) = match target {
FormatTarget::Buffers => {
let mut buffers = buffer.read(cx).all_buffers();
if trigger == FormatTrigger::Save {
buffers.retain(|buffer| buffer.read(cx).is_dirty());
}
(buffers, LspFormatTarget::Buffers)
}
FormatTarget::Ranges(selection_ranges) => {
let multi_buffer = buffer.read(cx);
let snapshot = multi_buffer.read(cx);
let mut buffers = HashSet::default();
let mut buffer_id_to_ranges: BTreeMap<BufferId, Vec<Range<text::Anchor>>> =
BTreeMap::new();
for selection_range in selection_ranges {
for (excerpt, buffer_range) in snapshot.range_to_buffer_ranges(selection_range)
{
let buffer_id = excerpt.buffer_id();
let start = excerpt.buffer().anchor_before(buffer_range.start);
let end = excerpt.buffer().anchor_after(buffer_range.end);
buffers.insert(multi_buffer.buffer(buffer_id).unwrap());
buffer_id_to_ranges
.entry(buffer_id)
.and_modify(|buffer_ranges| buffer_ranges.push(start..end))
.or_insert_with(|| vec![start..end]);
}
}
(buffers, LspFormatTarget::Ranges(buffer_id_to_ranges))
}
};

let mut timeout = cx.background_executor().timer(FORMAT_TIMEOUT).fuse();
let format = project.update(cx, |project, cx| {
project.format(buffers, true, trigger, target, cx)
project.format(buffers, target, true, trigger, cx)
});

cx.spawn(|_, mut cx| async move {
Expand Down
8 changes: 4 additions & 4 deletions crates/editor/src/editor_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7380,7 +7380,7 @@ async fn test_document_format_manual_trigger(cx: &mut gpui::TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffer,
FormatTarget::Buffers,
cx,
)
})
Expand Down Expand Up @@ -7418,7 +7418,7 @@ async fn test_document_format_manual_trigger(cx: &mut gpui::TestAppContext) {
});
let format = editor
.update(cx, |editor, cx| {
editor.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffer, cx)
editor.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffers, cx)
})
.unwrap();
cx.executor().advance_clock(super::FORMAT_TIMEOUT);
Expand Down Expand Up @@ -11293,7 +11293,7 @@ async fn test_document_format_with_prettier(cx: &mut gpui::TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffer,
FormatTarget::Buffers,
cx,
)
})
Expand All @@ -11312,7 +11312,7 @@ async fn test_document_format_with_prettier(cx: &mut gpui::TestAppContext) {
editor.perform_format(
project.clone(),
FormatTrigger::Manual,
FormatTarget::Buffer,
FormatTarget::Buffers,
cx,
)
});
Expand Down
7 changes: 3 additions & 4 deletions crates/editor/src/items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::{
editor_settings::SeedQuerySetting,
persistence::{SerializedEditor, DB},
scroll::ScrollAnchor,
Anchor, Autoscroll, Editor, EditorEvent, EditorSettings, ExcerptId, ExcerptRange, MultiBuffer,
MultiBufferSnapshot, NavigationData, SearchWithinRange, ToPoint as _,
Anchor, Autoscroll, Editor, EditorEvent, EditorSettings, ExcerptId, ExcerptRange, FormatTarget,
MultiBuffer, MultiBufferSnapshot, NavigationData, SearchWithinRange, ToPoint as _,
};
use anyhow::{anyhow, Context as _, Result};
use collections::HashSet;
Expand All @@ -29,7 +29,6 @@ use rpc::proto::{self, update_view, PeerId};
use settings::Settings;
use workspace::item::{Dedup, ItemSettings, SerializableItem, TabContentParams};

use project::lsp_store::FormatTarget;
use std::{
any::TypeId,
borrow::Cow,
Expand Down Expand Up @@ -756,7 +755,7 @@ impl Item for Editor {
editor.perform_format(
project.clone(),
FormatTrigger::Save,
FormatTarget::Buffer,
FormatTarget::Buffers,
cx,
)
})?
Expand Down
Loading

0 comments on commit 685dd77

Please sign in to comment.