From 5893e85708d05f58ca434e732759c7760bafbd14 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 29 Oct 2024 17:24:10 +0200 Subject: [PATCH] Ensure shared ssh project propagates buffer changes to all participants (#19907) Fixed the bug when shared ssh project did not account for client changing things in their buffers. Also ensures Prettier formatting workflow works for both ssh project owner and ssh project clients. Release Notes: - N/A --------- Co-authored-by: Conrad Irwin --- crates/collab/src/tests/integration_tests.rs | 6 +- .../remote_editing_collaboration_tests.rs | 197 +++++++++++++++++- crates/prettier/src/prettier.rs | 6 +- crates/project/src/lsp_store.rs | 3 +- crates/project/src/project.rs | 20 +- 5 files changed, 221 insertions(+), 11 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index c905c440cf829b3df10cf18700a4fac6adf6060c..b1e8e5686109a9a85c2950e68d5fab037f42f35e 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -21,8 +21,8 @@ use language::{ language_settings::{ AllLanguageSettings, Formatter, FormatterList, PrettierSettings, SelectedFormatter, }, - tree_sitter_rust, Diagnostic, DiagnosticEntry, FakeLspAdapter, Language, LanguageConfig, - LanguageMatcher, LineEnding, OffsetRangeExt, Point, Rope, + tree_sitter_rust, tree_sitter_typescript, Diagnostic, DiagnosticEntry, FakeLspAdapter, + Language, LanguageConfig, LanguageMatcher, LineEnding, OffsetRangeExt, Point, Rope, }; use live_kit_client::MacOSDisplay; use lsp::LanguageServerId; @@ -4461,7 +4461,7 @@ async fn test_prettier_formatting_buffer( }, ..Default::default() }, - Some(tree_sitter_rust::LANGUAGE.into()), + Some(tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into()), ))); let mut fake_language_servers = client_a.language_registry().register_fake_lsp( "TypeScript", diff --git a/crates/collab/src/tests/remote_editing_collaboration_tests.rs b/crates/collab/src/tests/remote_editing_collaboration_tests.rs index 9fe546ffcd125061f515676fef47cc972c67bb3a..0e29bd5ef3c9125a2f65e4cf9e35186229533fba 100644 --- a/crates/collab/src/tests/remote_editing_collaboration_tests.rs +++ b/crates/collab/src/tests/remote_editing_collaboration_tests.rs @@ -1,14 +1,27 @@ use crate::tests::TestServer; use call::ActiveCall; +use collections::HashSet; use fs::{FakeFs, Fs as _}; -use gpui::{BackgroundExecutor, Context as _, TestAppContext}; +use futures::StreamExt as _; +use gpui::{BackgroundExecutor, Context as _, TestAppContext, UpdateGlobal as _}; use http_client::BlockedHttpClient; -use language::{language_settings::language_settings, LanguageRegistry}; +use language::{ + language_settings::{ + language_settings, AllLanguageSettings, Formatter, FormatterList, PrettierSettings, + SelectedFormatter, + }, + tree_sitter_typescript, FakeLspAdapter, Language, LanguageConfig, LanguageMatcher, + LanguageRegistry, +}; use node_runtime::NodeRuntime; -use project::ProjectPath; +use project::{ + lsp_store::{FormatTarget, FormatTrigger}, + ProjectPath, +}; use remote::SshRemoteClient; use remote_server::{HeadlessAppState, HeadlessProject}; use serde_json::json; +use settings::SettingsStore; use std::{path::Path, sync::Arc}; #[gpui::test(iterations = 10)] @@ -304,3 +317,181 @@ async fn test_ssh_collaboration_git_branches( assert_eq!(server_branch.as_ref(), "totally-new-branch"); } + +#[gpui::test] +async fn test_ssh_collaboration_formatting_with_prettier( + executor: BackgroundExecutor, + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, + server_cx: &mut TestAppContext, +) { + cx_a.set_name("a"); + cx_b.set_name("b"); + server_cx.set_name("server"); + + let mut server = TestServer::start(executor.clone()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) + .await; + + let (opts, server_ssh) = SshRemoteClient::fake_server(cx_a, server_cx); + let remote_fs = FakeFs::new(server_cx.executor()); + let buffer_text = "let one = \"two\""; + let prettier_format_suffix = project::TEST_PRETTIER_FORMAT_SUFFIX; + remote_fs + .insert_tree("/project", serde_json::json!({ "a.ts": buffer_text })) + .await; + + let test_plugin = "test_plugin"; + let ts_lang = Arc::new(Language::new( + LanguageConfig { + name: "TypeScript".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["ts".to_string()], + ..LanguageMatcher::default() + }, + ..LanguageConfig::default() + }, + Some(tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into()), + )); + client_a.language_registry().add(ts_lang.clone()); + client_b.language_registry().add(ts_lang.clone()); + + let languages = Arc::new(LanguageRegistry::new(server_cx.executor())); + let mut fake_language_servers = languages.register_fake_lsp( + "TypeScript", + FakeLspAdapter { + prettier_plugins: vec![test_plugin], + ..Default::default() + }, + ); + + // User A connects to the remote project via SSH. + server_cx.update(HeadlessProject::init); + let remote_http_client = Arc::new(BlockedHttpClient); + let _headless_project = server_cx.new_model(|cx| { + client::init_settings(cx); + HeadlessProject::new( + HeadlessAppState { + session: server_ssh, + fs: remote_fs.clone(), + http_client: remote_http_client, + node_runtime: NodeRuntime::unavailable(), + languages, + }, + cx, + ) + }); + + let client_ssh = SshRemoteClient::fake_client(opts, cx_a).await; + let (project_a, worktree_id) = client_a + .build_ssh_project("/project", client_ssh, cx_a) + .await; + + // While the SSH worktree is being scanned, user A shares the remote project. + let active_call_a = cx_a.read(ActiveCall::global); + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + + // User B joins the project. + let project_b = client_b.join_remote_project(project_id, cx_b).await; + executor.run_until_parked(); + + // Opens the buffer and formats it + let buffer_b = project_b + .update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.ts"), cx)) + .await + .expect("user B opens buffer for formatting"); + + cx_a.update(|cx| { + SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings::(cx, |file| { + file.defaults.formatter = Some(SelectedFormatter::Auto); + file.defaults.prettier = Some(PrettierSettings { + allowed: true, + ..PrettierSettings::default() + }); + }); + }); + }); + cx_b.update(|cx| { + SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings::(cx, |file| { + file.defaults.formatter = Some(SelectedFormatter::List(FormatterList( + vec![Formatter::LanguageServer { name: None }].into(), + ))); + file.defaults.prettier = Some(PrettierSettings { + allowed: true, + ..PrettierSettings::default() + }); + }); + }); + }); + let fake_language_server = fake_language_servers.next().await.unwrap(); + fake_language_server.handle_request::(|_, _| async move { + panic!( + "Unexpected: prettier should be preferred since it's enabled and language supports it" + ) + }); + + project_b + .update(cx_b, |project, cx| { + project.format( + HashSet::from_iter([buffer_b.clone()]), + true, + FormatTrigger::Save, + FormatTarget::Buffer, + cx, + ) + }) + .await + .unwrap(); + + executor.run_until_parked(); + assert_eq!( + buffer_b.read_with(cx_b, |buffer, _| buffer.text()), + buffer_text.to_string() + "\n" + prettier_format_suffix, + "Prettier formatting was not applied to client buffer after client's request" + ); + + // User A opens and formats the same buffer too + let buffer_a = project_a + .update(cx_a, |p, cx| p.open_buffer((worktree_id, "a.ts"), cx)) + .await + .expect("user A opens buffer for formatting"); + + cx_a.update(|cx| { + SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings::(cx, |file| { + file.defaults.formatter = Some(SelectedFormatter::Auto); + file.defaults.prettier = Some(PrettierSettings { + allowed: true, + ..PrettierSettings::default() + }); + }); + }); + }); + project_a + .update(cx_a, |project, cx| { + project.format( + HashSet::from_iter([buffer_a.clone()]), + true, + FormatTrigger::Manual, + FormatTarget::Buffer, + cx, + ) + }) + .await + .unwrap(); + + executor.run_until_parked(); + assert_eq!( + buffer_b.read_with(cx_b, |buffer, _| buffer.text()), + buffer_text.to_string() + "\n" + prettier_format_suffix + "\n" + prettier_format_suffix, + "Prettier formatting was not applied to client buffer after host's request" + ); +} diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index d2d56696a696e3b901955dc70a98b3d201e50cf5..4dc5bca40f13e436853362aa5f3fb6c45a6b4a57 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -14,14 +14,14 @@ use std::{ }; use util::paths::PathMatcher; -#[derive(Clone)] +#[derive(Debug, Clone)] pub enum Prettier { Real(RealPrettier), #[cfg(any(test, feature = "test-support"))] Test(TestPrettier), } -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct RealPrettier { default: bool, prettier_dir: PathBuf, @@ -29,7 +29,7 @@ pub struct RealPrettier { } #[cfg(any(test, feature = "test-support"))] -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct TestPrettier { prettier_dir: PathBuf, default: bool, diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 40e87b55e5b81bd3a74ffd3e8821daca4a1d5706..fe39dc09141866a3dd6f0cc8ccf0ef27319b0948 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -675,6 +675,7 @@ impl LocalLspStore { } } +#[derive(Debug)] pub struct FormattableBuffer { handle: Model, abs_path: Option, @@ -5342,7 +5343,7 @@ impl LspStore { buffers.insert(this.buffer_store.read(cx).get_existing(buffer_id)?); } let trigger = FormatTrigger::from_proto(envelope.payload.trigger); - Ok::<_, anyhow::Error>(this.format(buffers, false, trigger, FormatTarget::Buffer, cx)) + anyhow::Ok(this.format(buffers, false, trigger, FormatTarget::Buffer, cx)) })??; let project_transaction = format.await?; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 7fd77fb0ad26f58e023f9dca9b874423300cf191..f5a295a3a3fb6d214dd06351b30ab01066d7e18b 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -827,7 +827,7 @@ impl Project { ssh_proto.add_model_message_handler(Self::handle_toast); ssh_proto.add_model_request_handler(Self::handle_language_server_prompt_request); ssh_proto.add_model_message_handler(Self::handle_hide_toast); - ssh_proto.add_model_request_handler(BufferStore::handle_update_buffer); + ssh_proto.add_model_request_handler(Self::handle_update_buffer_from_ssh); BufferStore::init(&ssh_proto); LspStore::init(&ssh_proto); SettingsObserver::init(&ssh_proto); @@ -3653,6 +3653,24 @@ impl Project { })? } + async fn handle_update_buffer_from_ssh( + this: Model, + envelope: TypedEnvelope, + cx: AsyncAppContext, + ) -> Result { + let buffer_store = this.read_with(&cx, |this, cx| { + if let Some(remote_id) = this.remote_id() { + let mut payload = envelope.payload.clone(); + payload.project_id = remote_id; + cx.background_executor() + .spawn(this.client.request(payload)) + .detach_and_log_err(cx); + } + this.buffer_store.clone() + })?; + BufferStore::handle_update_buffer(buffer_store, envelope, cx).await + } + async fn handle_update_buffer( this: Model, envelope: TypedEnvelope,