From 24254879b711ffa286a508db0c7060a434aa20bb Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 31 Aug 2026 11:58:50 +0000 Subject: [PATCH] Stop re-resolving editorconfig sections on every language settings lookup (#63323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/zed-industries/zed/issues/59689 Symbolicating the reporter's profile (21s of typing, Windows) showed `Window::draw` taking 63% of the main thread at ~62 redraws/s (p95 draw = 40ms), and 30.9% of the *entire* main thread inside `EditorconfigStore::properties()`: every `LanguageSettings` lookup — several per frame via `DisplayMap::tab_size`, `MultiBuffer::language_settings`, indent guides, and `LspStore::supports_range_formatting` — re-ran the full editorconfig resolution (ancestor walk plus `ec4rs::Section::apply_to` over every key of every section) and then cloned the whole `LanguageSettings` struct. Real-world `.editorconfig` files carry hundreds of irrelevant keys (`dotnet_*`, `ij_*`, ...), so the per-call map assembly dominated: `RtlAllocateHeap` 7.3%, `memcmp` 7.0%, `memcpy` 6.6%, `Properties::find_idx` 5.9% self time. ## Commit 1: resolve only the editorconfig properties Zed reads - `EditorconfigStore::properties()` now applies only the 7 keys Zed consumes (`indent_style`, `indent_size`, `tab_width`, `end_of_line`, `max_line_length`, `insert_final_newline`, `trim_trailing_whitespace`) instead of materializing every key of every matching section. - Worktrees with no editorconfig at all now return `None` early instead of allocating an empty `Properties` and forcing a `LanguageSettings` clone on every lookup. - Adds an `editor_render_with_editorconfig` benchmark: maximized window, file-backed buffer at `src/app/components/editor_pane.ts`, a root `.editorconfig` shaped after [the second report](https://github.com/zed-industries/zed/issues/59689#issuecomment-5475124891) (no `root = true`, a `[*]` section with ~3200 keys, 25 sections with large `{...}` alternation lists) plus a 4-level nested chain, and a typing workload (insert/delete + cursor moves). ``` main (770aac345c): time: [37.905 ms 38.005 ms 38.107 ms] time: [38.069 ms 38.157 ms 38.248 ms] this commit: time: [2.8547 ms 2.8618 ms 2.8702 ms] time: [2.8372 ms 2.8435 ms 2.8510 ms] ``` The candidate is ~13x faster. GPUI frame report over the same workload: window draw p50 12.71 ms → 0.76 ms, p99 23.38 ms → 1.22 ms; frame budget overruns 556 → 0. ## Commit 2: cache resolved `LanguageSettings` per buffer - `Buffer` and `BufferSnapshot` now hold `resolved_settings: Arc`, recomputed by a `SettingsStore` global observer. - Global methods for retrieving the settings are still there, but buffer only uses them for multi buffer's `language_settings_at` with an injected language different from the buffer's primary; cold scopes such as extension API, LSP workspace configuration, server startup still use the global resolution Same benchmark, same method, on top of commit 1 (alternated runs, quiet machine): ``` commit 1: time: [2.8547 ms 2.8618 ms 2.8702 ms] time: [2.8372 ms 2.8435 ms 2.8510 ms] this commit: time: [1.6256 ms 1.6325 ms 1.6403 ms] change: [-42.685% -42.270% -41.854%] (p = 0.00 < 0.05) time: [1.6221 ms 1.6284 ms 1.6352 ms] ``` Window draw p50 0.76 ms → 0.47 ms, p99 1.22 ms → 0.69 ms, max 1.02 ms; zero frame-budget overruns on both sides. Combined, the benchmark's per-keystroke cost drops ~23x vs `main` (37.9–38.2 ms → 1.62–1.64 ms). Release Notes: - Improved typing and rendering performance in projects that use `.editorconfig` files. --- Cargo.lock | 2 + crates/acp_thread/src/diff.rs | 2 +- crates/benchmarks/Cargo.toml | 2 + crates/benchmarks/benches/editor_render.rs | 237 +++++++++++++++++- crates/channel/src/channel_buffer.rs | 1 + .../src/edit_prediction_tests.rs | 4 +- crates/editor/src/editor.rs | 13 +- crates/editor/src/rewrap.rs | 2 +- crates/git_ui/src/commit_view.rs | 2 +- crates/language/src/buffer.rs | 76 +++++- crates/language/src/buffer_tests.rs | 78 +++++- crates/language/src/language_settings.rs | 99 +++++--- crates/multi_buffer/src/multi_buffer.rs | 22 +- crates/multi_buffer/src/multi_buffer_tests.rs | 11 +- crates/project/src/buffer_store.rs | 19 +- crates/project/src/lsp_store.rs | 57 +++-- crates/project/src/prettier_store.rs | 25 -- .../tests/integration/project_tests.rs | 12 +- crates/settings/src/editorconfig_store.rs | 155 +++++++++++- 19 files changed, 671 insertions(+), 148 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bcb5c8012a..9087886dd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2135,6 +2135,7 @@ dependencies = [ "futures 0.3.32", "gpui", "gpui_platform", + "indoc", "itertools 0.14.0", "language", "language_model", @@ -2144,6 +2145,7 @@ dependencies = [ "project", "prompt_store", "rand 0.9.4", + "rpc", "serde_json", "settings", "text", diff --git a/crates/acp_thread/src/diff.rs b/crates/acp_thread/src/diff.rs index 72f310f7f3..d238d22c19 100644 --- a/crates/acp_thread/src/diff.rs +++ b/crates/acp_thread/src/diff.rs @@ -279,7 +279,7 @@ impl PendingDiff { self.new_buffer.read(cx).line_ending(), self.new_buffer.read(cx).as_rope().clone(), ); - let mut buffer = Buffer::build(buffer, None, Capability::ReadWrite); + let mut buffer = Buffer::build(buffer, None, Capability::ReadWrite, cx); buffer.set_language(language, cx); buffer }); diff --git a/crates/benchmarks/Cargo.toml b/crates/benchmarks/Cargo.toml index 95756c7d1c..2415ebc396 100644 --- a/crates/benchmarks/Cargo.toml +++ b/crates/benchmarks/Cargo.toml @@ -25,6 +25,7 @@ editor = { workspace = true, features = ["test-support"] } futures.workspace = true gpui = { workspace = true, features = ["bench-support"] } gpui_platform = { workspace = true, features = ["bench-support", "font-kit"] } +indoc.workspace = true itertools.workspace = true language = { workspace = true, features = ["test-support"] } language_model = { workspace = true, features = ["test-support"] } @@ -33,6 +34,7 @@ markdown.workspace = true multi_buffer.workspace = true project = { workspace = true, features = ["test-support"] } prompt_store.workspace = true +rpc.workspace = true serde_json.workspace = true settings = { workspace = true, features = ["test-support"] } text.workspace = true diff --git a/crates/benchmarks/benches/editor_render.rs b/crates/benchmarks/benches/editor_render.rs index 67433bdb22..695cab7e8f 100644 --- a/crates/benchmarks/benches/editor_render.rs +++ b/crates/benchmarks/benches/editor_render.rs @@ -1,13 +1,59 @@ +use std::{path::PathBuf, sync::Arc}; + use editor::{ Editor, EditorMode, MultiBuffer, actions::{DeleteToPreviousWordStart, SelectAll, SplitSelectionIntoLines}, }; -use gpui::{AppContext as _, BenchAppContext, Focusable as _}; +use gpui::{App, AppContext as _, BenchAppContext, BorrowAppContext as _, Focusable as _}; +use indoc::{formatdoc, indoc}; +use language::{Buffer, Capability, DiskState, File, LocalFile}; use rand::{Rng as _, SeedableRng as _, rngs::StdRng}; -use settings::SettingsStore; -use util::RandomCharIter; +use settings::{LocalSettingsKind, LocalSettingsPath, SettingsStore, WorktreeId}; +use util::{RandomCharIter, paths::PathStyle, rel_path::RelPath}; use zed_actions::editor::{MoveDown, MoveUp}; +struct BenchFile { + path: Arc, +} + +impl File for BenchFile { + fn as_local(&self) -> Option<&dyn LocalFile> { + None + } + + fn disk_state(&self) -> DiskState { + DiskState::New + } + + fn path(&self) -> &Arc { + &self.path + } + + fn full_path(&self, _: &App) -> PathBuf { + PathBuf::from("root").join(self.path.as_std_path()) + } + + fn path_style(&self, _: &App) -> PathStyle { + PathStyle::local() + } + + fn file_name<'a>(&'a self, _: &'a App) -> &'a str { + self.path.file_name().unwrap_or("root") + } + + fn worktree_id(&self, _: &App) -> WorktreeId { + WorktreeId::from_usize(0) + } + + fn to_proto(&self, _: &App) -> rpc::proto::File { + unimplemented!() + } + + fn is_private(&self) -> bool { + false + } +} + #[gpui::bench( inputs = multi_cursor_line_counts(), group = "Multi-cursor input", @@ -124,6 +170,188 @@ fn editor_render(cx: &mut BenchAppContext) { }); } +#[gpui::bench] +fn editor_render_with_editorconfig(cx: &mut BenchAppContext) { + init_context(cx); + + let worktree_id = WorktreeId::from_usize(0); + cx.update(|cx| { + cx.update_global::(|store, cx| { + let nested_configs = [ + ("", jetbrains_editorconfig()), + ( + "src", + indoc! {" + [*.{ts,tsx}] + indent_size = 2 + "} + .to_string(), + ), + ( + "src/app", + indoc! {" + [*] + trim_trailing_whitespace = false + + [*.ts] + max_line_length = 100 + "} + .to_string(), + ), + ( + "src/app/components", + indoc! {" + [*.{ts,tsx}] + indent_style = space + "} + .to_string(), + ), + ]; + for (directory, content) in nested_configs { + store + .set_local_settings( + worktree_id, + LocalSettingsPath::InWorktree(Arc::from( + RelPath::from_unix_str(directory).unwrap(), + )), + LocalSettingsKind::Editorconfig, + Some(&content), + cx, + ) + .unwrap(); + } + }); + }); + + let buffer = cx.update(|cx| { + let text = indented_code_text(3000); + let file: Arc = Arc::new(BenchFile { + path: RelPath::from_unix_str("src/app/components/editor_pane.ts") + .unwrap() + .into(), + }); + let buffer = cx.new(|cx| { + Buffer::build( + text::Buffer::new( + text::ReplicaId::LOCAL, + cx.entity_id().as_non_zero_u64().into(), + text, + ), + Some(file), + Capability::ReadWrite, + cx, + ) + }); + cx.new(|cx| MultiBuffer::singleton(buffer, cx)) + }); + + let mut window = cx.add_empty_window(); + let editor = window.update(|window, cx| { + let editor = window.replace_root(cx, |window, cx| { + let mut editor = Editor::new(EditorMode::full(), buffer, None, window, cx); + editor.set_style(editor::EditorStyle::default(), window, cx); + editor + }); + window.focus(&editor.focus_handle(cx), cx); + editor + }); + + let mut insert = true; + cx.bench_renderer(editor, move |editor, window, cx| { + if insert { + editor.handle_input("x", window, cx); + } else { + editor.backspace(&editor::actions::Backspace, window, cx); + } + insert = !insert; + editor.move_down(&MoveDown, window, cx); + editor.move_up(&MoveUp, window, cx); + }); +} + +fn indented_code_text(line_count: usize) -> String { + let mut text = String::new(); + for block in 0..line_count / 10 { + text.push_str(&format!("export function component{block:04}() {{\n")); + text.push_str(" const state = {\n"); + text.push_str(" items: [],\n"); + text.push_str(" selection: null,\n"); + text.push_str(" };\n"); + text.push_str(" if (state.items.length > 0) {\n"); + text.push_str(" for (const item of state.items) {\n"); + text.push_str(" console.log(item, state.selection);\n"); + text.push_str(" }\n"); + text.push_str(" }\n"); + text.push_str("}\n"); + } + text +} + +fn jetbrains_editorconfig() -> String { + let mut content = indoc! {" + [*] + charset = utf-8 + end_of_line = lf + indent_size = 4 + indent_style = space + insert_final_newline = true + max_line_length = 150 + tab_width = 4 + trim_trailing_whitespace = false + "} + .to_string(); + for key_index in 0..750 { + content.push_str(&format!("ij_continuation_option_{key_index:04} = false\n")); + } + for key_index in 0..1500 { + content.push_str(&format!( + "dotnet_diagnostic.ca{key_index:04}.severity = warning\n" + )); + } + for key_index in 0..750 { + content.push_str(&format!("resharper_style_option_{key_index:04} = true\n")); + } + let sections = [ + "*.css", + "*.feature", + "*.less", + "*.properties", + "*.proto", + "*.sass", + "*.scss", + "*.vue", + ".editorconfig", + "{*.ant,*.appxmanifest,*.axml,*.cscfg,*.csdef,*.disco,*.filelayout,*.fxml,*.jhm,*.jnlp,*.jrxml,*.manifest,*.myapp,*.nuspec,*.rng,*.stylecop,*.svcmap,*.tld,*.tps,*.wadcfgx,*.webref,*.wsdl,*.xml,*.xsd,*.xsl,*.xslt,*.xul,StyleCop.Cache}", + "{*.ats,*.ts}", + "{*.bash,*.sh,*.zsh}", + "{*.cjs,*.js}", + "{*.cjsx,*.coffee}", + "{*.har,*.inputactions,*.jsb2,*.jsb3,*.json,.babelrc,.eslintrc,.stylelintrc,bowerrc,jest.config}", + "{*.hcl,*.nomad}", + "{*.htm,*.html,*.ng,*.sht,*.shtm,*.shtml}", + "{*.markdown,*.md}", + "{*.pb,*.textproto}", + "{*.ps1,*.psd1,*.psm1}", + "{*.tf,*.tfvars}", + "{*.yaml,*.yml}", + "*.js.map", + "*.{appxmanifest,asax,ascx,aspx,axaml,build,cg,cginc,compute,cs,cshtml,dtd,fs,fsi,fsscript,fsx,hlsl,hlsli,hlslinc,master,ml,mli,nuspec,paml,razor,resw,resx,shader,skin,usf,ush,vb,xaml,xamlx,xoml,xsd}", + ]; + for (section_index, section) in sections.iter().enumerate() { + content.push_str(&formatdoc! {" + + [{section}] + indent_size = 2 + "}); + for key_index in 0..40 { + content.push_str(&format!( + "ij_section_{section_index:02}_option_{key_index:02} = false\n" + )); + } + } + content +} + fn init_context(cx: &mut BenchAppContext) { cx.update(|cx| { let store = SettingsStore::test(cx); @@ -146,6 +374,7 @@ gpui::bench_group!( benches, editor_multi_cursor_input, open_editor_with_one_long_line, - editor_render + editor_render, + editor_render_with_editorconfig ); gpui::bench_main!(benches); diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index 6145b1cf05..52bb5e9a59 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/crates/channel/src/channel_buffer.rs @@ -71,6 +71,7 @@ impl ChannelBuffer { ReplicaId::new(response.replica_id as u16), capability, base_text, + cx, ) }); buffer.update(cx, |buffer, cx| buffer.apply_ops(operations, cx)); diff --git a/crates/edit_prediction/src/edit_prediction_tests.rs b/crates/edit_prediction/src/edit_prediction_tests.rs index 3d4212c964..c11542125e 100644 --- a/crates/edit_prediction/src/edit_prediction_tests.rs +++ b/crates/edit_prediction/src/edit_prediction_tests.rs @@ -767,8 +767,8 @@ fn make_collaborator_replica( ) -> (Entity, clock::Global) { let (state, version) = buffer.read_with(cx, |buffer, _cx| (buffer.to_proto(_cx), buffer.version())); - let collaborator = cx.new(|_cx| { - Buffer::from_proto(ReplicaId::new(1), Capability::ReadWrite, state, None).unwrap() + let collaborator = cx.new(|cx| { + Buffer::from_proto(ReplicaId::new(1), Capability::ReadWrite, state, None, cx).unwrap() }); (collaborator, version) } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 19fd9dec76..e04d28073c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1184,7 +1184,7 @@ pub struct Editor { on_local_selections_changed: Option) + 'static>>, suppress_selection_callback: bool, - applicable_language_settings: HashMap, LanguageSettings>, + applicable_language_settings: HashMap, Arc>, accent_data: Option, bracket_fetched_tree_sitter_chunks: HashMap, HashSet>>, semantic_token_state: SemanticTokenState, @@ -9961,6 +9961,13 @@ impl Editor { self.update_edit_prediction_settings(cx); cx.notify(); } + multi_buffer::Event::SettingsChanged => { + let new_language_settings = self.fetch_applicable_language_settings(cx); + if new_language_settings != self.applicable_language_settings { + self.applicable_language_settings = new_language_settings; + cx.notify(); + } + } multi_buffer::Event::DirtyChanged => cx.emit(EditorEvent::DirtyChanged), multi_buffer::Event::Saved => cx.emit(EditorEvent::Saved), multi_buffer::Event::FileHandleChanged => { @@ -10033,7 +10040,7 @@ impl Editor { fn fetch_applicable_language_settings( &self, cx: &App, - ) -> HashMap, LanguageSettings> { + ) -> HashMap, Arc> { if !self.mode.is_full() { return HashMap::default(); } @@ -10044,7 +10051,7 @@ impl Editor { let buffer = buffer.read(cx); let language = buffer.language().map(|language| language.name()); if let hash_map::Entry::Vacant(v) = acc.entry(language) { - v.insert(LanguageSettings::for_buffer(&buffer, cx).into_owned()); + v.insert(LanguageSettings::for_buffer(buffer, cx)); } acc }, diff --git a/crates/editor/src/rewrap.rs b/crates/editor/src/rewrap.rs index e8df9d7cc4..3b3ccd4636 100644 --- a/crates/editor/src/rewrap.rs +++ b/crates/editor/src/rewrap.rs @@ -204,7 +204,7 @@ impl Editor { } ranges.push(( - language_settings.clone(), + language_settings, Point::new(current_range_start, 0) ..Point::new(prev_row, buffer.line_len(MultiBufferRow(prev_row))), current_range_indent, diff --git a/crates/git_ui/src/commit_view.rs b/crates/git_ui/src/commit_view.rs index 027a614dd3..f9ce69c684 100644 --- a/crates/git_ui/src/commit_view.rs +++ b/crates/git_ui/src/commit_view.rs @@ -1139,7 +1139,7 @@ pub(crate) async fn build_buffer( line_ending, text, ); - let mut buffer = Buffer::build(buffer, Some(blob), Capability::ReadWrite); + let mut buffer = Buffer::build(buffer, Some(blob), Capability::ReadWrite, cx); buffer.set_language_async(language, cx); buffer }); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 9c6321addb..5bf8e87112 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -38,11 +38,10 @@ use gpui::{ use lsp::LanguageServerId; use parking_lot::Mutex; -use settings::WorktreeId; +use settings::{SettingsStore, WorktreeId}; use smallvec::SmallVec; use std::{ any::Any, - borrow::Cow, cell::Cell, cmp::{self, Ordering, Reverse}, collections::{BTreeMap, BTreeSet}, @@ -139,6 +138,8 @@ pub struct Buffer { change_bits: Vec>>, modeline: Option>, _subscriptions: Vec, + resolved_settings: Option>, + _settings_observer: Option, tree_sitter_data: Arc, encoding: &'static Encoding, has_bom: bool, @@ -195,6 +196,7 @@ pub struct BufferSnapshot { non_text_state_update_count: usize, pub capability: Capability, modeline: Option>, + resolved_settings: Option>, } /// The kind and amount of indentation in a particular line. For now, @@ -337,6 +339,8 @@ pub enum BufferEvent { LanguageChanged(bool), /// The buffer's syntax trees were updated. Reparsed, + /// The buffer's resolved language settings were changed. + SettingsChanged, /// The buffer's diagnostics were updated. DiagnosticsUpdated, /// The buffer gained or lost editing capabilities. @@ -963,7 +967,7 @@ pub enum AutoIndentExclusion { impl Buffer { /// Create a new buffer with the given base text. - pub fn local>(base_text: T, cx: &Context) -> Self { + pub fn local>(base_text: T, cx: &mut Context) -> Self { Self::build( TextBuffer::new( ReplicaId::LOCAL, @@ -972,6 +976,7 @@ impl Buffer { ), None, Capability::ReadWrite, + cx, ) } @@ -979,7 +984,7 @@ impl Buffer { pub fn local_normalized( base_text_normalized: Rope, line_ending: LineEnding, - cx: &Context, + cx: &mut Context, ) -> Self { Self::build( TextBuffer::new_normalized( @@ -990,6 +995,7 @@ impl Buffer { ), None, Capability::ReadWrite, + cx, ) } @@ -999,11 +1005,13 @@ impl Buffer { replica_id: ReplicaId, capability: Capability, base_text: impl Into, + cx: &mut Context, ) -> Self { Self::build( TextBuffer::new(replica_id, remote_id, base_text.into()), None, capability, + cx, ) } @@ -1014,10 +1022,11 @@ impl Buffer { capability: Capability, message: proto::BufferState, file: Option>, + cx: &mut Context, ) -> Result { let buffer_id = BufferId::new(message.id).context("Could not deserialize buffer_id")?; let buffer = TextBuffer::new(replica_id, buffer_id, message.base_text); - let mut this = Self::build(buffer, file, capability); + let mut this = Self::build(buffer, file, capability, cx); this.text.set_line_ending(proto::deserialize_line_ending( rpc::proto::LineEnding::try_from(message.line_ending) .ok() @@ -1114,12 +1123,17 @@ impl Buffer { } /// Builds a [`Buffer`] with the given underlying [`TextBuffer`], diff base, [`File`] and [`Capability`]. - pub fn build(buffer: TextBuffer, file: Option>, capability: Capability) -> Self { + pub fn build( + buffer: TextBuffer, + file: Option>, + capability: Capability, + cx: &mut Context, + ) -> Self { let saved_mtime = file.as_ref().and_then(|file| file.disk_state().mtime()); let snapshot = buffer.snapshot(); let syntax_map = Mutex::new(SyntaxMap::new(&snapshot)); let tree_sitter_data = TreeSitterData::new(snapshot); - Self { + let mut this = Self { saved_mtime, tree_sitter_data: Arc::new(tree_sitter_data), saved_version: buffer.version(), @@ -1157,12 +1171,38 @@ impl Buffer { change_bits: Default::default(), modeline: None, _subscriptions: Vec::new(), + resolved_settings: None, + _settings_observer: Some(cx.observe_global::(|this, cx| { + this.refresh_resolved_settings(cx); + })), encoding: encoding_rs::UTF_8, has_bom: false, reload_with_encoding_txns: HashMap::default(), + }; + this.resolved_settings = this.compute_resolved_settings(cx); + this + } + + fn compute_resolved_settings(&self, cx: &App) -> Option> { + cx.try_global::()?; + Some(LanguageSettings::resolve_uncached(self, None, cx)) + } + + fn refresh_resolved_settings(&mut self, cx: &mut Context) { + let resolved = self.compute_resolved_settings(cx); + if resolved.as_deref() != self.resolved_settings.as_deref() { + self.resolved_settings = resolved; + self.non_text_state_update_count += 1; + self.was_changed(); + cx.emit(BufferEvent::SettingsChanged); + cx.notify(); } } + pub(crate) fn resolved_settings(&self) -> Option<&Arc> { + self.resolved_settings.as_ref() + } + #[ztracing::instrument(skip_all)] pub fn build_snapshot( text: Rope, @@ -1194,6 +1234,7 @@ impl Buffer { non_text_state_update_count: 0, capability: Capability::ReadOnly, modeline, + resolved_settings: None, } } } @@ -1221,6 +1262,7 @@ impl Buffer { non_text_state_update_count: 0, capability: Capability::ReadOnly, modeline: None, + resolved_settings: None, } } @@ -1252,6 +1294,7 @@ impl Buffer { non_text_state_update_count: 0, capability: Capability::ReadOnly, modeline: None, + resolved_settings: None, } } @@ -1283,6 +1326,7 @@ impl Buffer { non_text_state_update_count: self.non_text_state_update_count, capability: self.capability, modeline: self.modeline.clone(), + resolved_settings: self.resolved_settings.clone(), } } @@ -1299,7 +1343,7 @@ impl Buffer { has_conflict: self.has_conflict, has_unsaved_edits: Cell::new(self.has_unsaved_edits.get_mut().clone()), _subscriptions: vec![cx.subscribe(&this, Self::on_base_buffer_event)], - ..Self::build(self.text.branch(), self.file.clone(), self.capability()) + ..Self::build(self.text.branch(), self.file.clone(), self.capability(), cx) }; if let Some(language_registry) = self.language_registry() { branch.set_language_registry(language_registry); @@ -1510,6 +1554,7 @@ impl Buffer { self.non_text_state_update_count += 1; self.syntax_map.lock().clear(&self.text); let old_language = std::mem::replace(&mut self.language, language); + self.refresh_resolved_settings(cx); self.was_changed(); self.reparse(cx, may_block); let has_fresh_language = @@ -1545,9 +1590,14 @@ impl Buffer { } /// Assign the buffer [`ModelineSettings`]. - pub fn set_modeline(&mut self, modeline: Option) -> bool { + pub fn set_modeline( + &mut self, + modeline: Option, + cx: &mut Context, + ) -> bool { if modeline.as_ref() != self.modeline.as_deref() { self.modeline = modeline.map(Arc::new); + self.refresh_resolved_settings(cx); true } else { false @@ -1733,6 +1783,7 @@ impl Buffer { self.file = Some(new_file); if file_changed { + self.refresh_resolved_settings(cx); self.was_changed(); self.non_text_state_update_count += 1; if was_dirty != self.is_dirty() { @@ -4177,6 +4228,10 @@ impl BufferSnapshot { self.modeline.as_ref() } + pub(crate) fn resolved_settings(&self) -> Option<&Arc> { + self.resolved_settings.as_ref() + } + /// Returns the main [`Language`]. pub fn language(&self) -> Option<&Arc> { self.language.as_ref() @@ -4194,7 +4249,7 @@ impl BufferSnapshot { &'a self, position: D, cx: &'a App, - ) -> Cow<'a, LanguageSettings> { + ) -> Arc { LanguageSettings::for_buffer_snapshot(self, Some(position.to_offset(self)), cx) } @@ -5430,6 +5485,7 @@ impl Clone for BufferSnapshot { non_text_state_update_count: self.non_text_state_update_count, capability: self.capability, modeline: self.modeline.clone(), + resolved_settings: self.resolved_settings.clone(), } } } diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 01d028fcea..80646668b0 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -76,6 +76,7 @@ fn test_set_line_ending(cx: &mut TestAppContext) { Capability::ReadWrite, base.read(cx).to_proto(cx), None, + cx, ) .unwrap() }); @@ -574,6 +575,7 @@ fn test_edit_events(cx: &mut gpui::App) { ReplicaId::new(1), Capability::ReadWrite, "abcdef", + cx, ) }); let buffer1_ops = Arc::new(Mutex::new(Vec::new())); @@ -3844,7 +3846,7 @@ fn test_serialization(cx: &mut gpui::App) { .block_on(buffer1.read(cx).serialize_ops(None, cx)); let buffer2 = cx.new(|cx| { let mut buffer = - Buffer::from_proto(ReplicaId::new(1), Capability::ReadWrite, state, None).unwrap(); + Buffer::from_proto(ReplicaId::new(1), Capability::ReadWrite, state, None, cx).unwrap(); buffer.apply_ops( ops.into_iter() .map(|op| proto::deserialize_operation(op).unwrap()), @@ -3868,6 +3870,7 @@ fn test_branch_and_merge(cx: &mut TestAppContext) { Capability::ReadWrite, base.read(cx).to_proto(cx), None, + cx, ) .unwrap() }); @@ -4180,9 +4183,14 @@ fn test_random_collaboration(cx: &mut App, mut rng: StdRng) { let ops = cx .foreground_executor() .block_on(base_buffer.read(cx).serialize_ops(None, cx)); - let mut buffer = - Buffer::from_proto(ReplicaId::new(i as u16), Capability::ReadWrite, state, None) - .unwrap(); + let mut buffer = Buffer::from_proto( + ReplicaId::new(i as u16), + Capability::ReadWrite, + state, + None, + cx, + ) + .unwrap(); buffer.apply_ops( ops.into_iter() .map(|op| proto::deserialize_operation(op).unwrap()), @@ -4311,6 +4319,7 @@ fn test_random_collaboration(cx: &mut App, mut rng: StdRng) { Capability::ReadWrite, old_buffer_state, None, + cx, ) .unwrap(); new_buffer.apply_ops( @@ -5194,6 +5203,7 @@ fn test_completion_triggers_across_language_servers(cx: &mut TestAppContext) { Capability::ReadWrite, buffer.read(cx).to_proto(cx), None, + cx, ) .unwrap() }); @@ -5317,6 +5327,66 @@ fn init_settings(cx: &mut App, f: fn(&mut AllLanguageSettingsContent)) { }); } +#[gpui::test] +fn test_settings_changed_event(cx: &mut TestAppContext) { + cx.update(|cx| init_settings(cx, |_| {})); + + let buffer = cx.new(|cx| Buffer::local("one\ntwo\nthree\n", cx)); + let settings_change_count = std::rc::Rc::new(std::cell::Cell::new(0)); + let subscription = cx.update(|cx| { + cx.subscribe(&buffer, { + let settings_change_count = settings_change_count.clone(); + move |_, event, _| { + if let BufferEvent::SettingsChanged = event { + settings_change_count.set(settings_change_count.get() + 1); + } + } + }) + }); + + assert_eq!( + buffer.read_with(cx, |buffer, cx| { + crate::language_settings::LanguageSettings::for_buffer(buffer, cx) + .tab_size + .get() + }), + 4 + ); + assert_eq!(settings_change_count.get(), 0); + + cx.update(|cx| { + cx.update_global::(|settings, cx| { + settings.update_user_settings(cx, |content| { + content.project.all_languages.defaults.tab_size = Some(3.try_into().unwrap()); + }); + }); + }); + assert_eq!(settings_change_count.get(), 1); + assert_eq!( + buffer.read_with(cx, |buffer, cx| { + crate::language_settings::LanguageSettings::for_buffer(buffer, cx) + .tab_size + .get() + }), + 3 + ); + + cx.update(|cx| { + cx.update_global::(|settings, cx| { + settings.update_user_settings(cx, |content| { + content.project.all_languages.defaults.tab_size = Some(3.try_into().unwrap()); + }); + }); + }); + assert_eq!( + settings_change_count.get(), + 1, + "a no-op settings update should not emit SettingsChanged" + ); + + drop(subscription); +} + #[gpui::test(iterations = 100)] fn test_random_chunk_bitmaps(cx: &mut App, mut rng: StdRng) { use util::RandomCharIter; diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 7a492279e3..95c65f562d 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -24,7 +24,7 @@ pub use settings::{ }; use settings::{RegisterSetting, Settings, SettingsLocation, SettingsStore, merge_from::MergeFrom}; use shellexpand; -use std::{borrow::Cow, num::NonZeroU32, path::Path, sync::Arc, time::Duration}; +use std::{num::NonZeroU32, path::Path, sync::Arc, time::Duration}; use text::ToOffset; /// Returns the settings for all languages from the provided file. @@ -44,8 +44,8 @@ pub fn all_language_settings<'a>( pub struct AllLanguageSettings { /// The edit prediction settings. pub edit_predictions: EditPredictionSettings, - pub defaults: LanguageSettings, - languages: HashMap, + pub defaults: Arc, + languages: HashMap>, pub file_types: FxHashMap, (GlobSet, Vec)>, } @@ -277,35 +277,41 @@ pub struct PrettierSettings { } impl LanguageSettings { - pub fn for_buffer<'a>(buffer: &'a Buffer, cx: &'a App) -> Cow<'a, LanguageSettings> { + pub fn for_buffer(buffer: &Buffer, cx: &App) -> Arc { Self::resolve(Some(buffer), None, cx) } - pub fn for_buffer_at<'a, D: ToOffset>( - buffer: &'a Buffer, + pub fn for_buffer_at( + buffer: &Buffer, position: D, - cx: &'a App, - ) -> Cow<'a, LanguageSettings> { + cx: &App, + ) -> Arc { let language = buffer.language_at(position); Self::resolve(Some(buffer), language.map(|l| l.name()).as_ref(), cx) } - pub fn for_buffer_snapshot<'a>( - buffer: &'a BufferSnapshot, + pub fn for_buffer_snapshot( + buffer: &BufferSnapshot, offset: Option, - cx: &'a App, - ) -> Cow<'a, LanguageSettings> { - let location = buffer.file().map(|f| SettingsLocation { - worktree_id: f.worktree_id(cx), - path: f.path().as_ref(), - }); - + cx: &App, + ) -> Arc { let language = if let Some(offset) = offset { buffer.language_at(offset) } else { buffer.language() }; + if let Some(resolved) = buffer.resolved_settings() + && language == buffer.language() + { + return Arc::clone(resolved); + } + + let location = buffer.file().map(|f| SettingsLocation { + worktree_id: f.worktree_id(cx), + path: f.path().as_ref(), + }); + let mut settings = AllLanguageSettings::get(location, cx).language( location, language.map(|l| l.name()).as_ref(), @@ -313,33 +319,48 @@ impl LanguageSettings { ); if let Some(modeline) = buffer.modeline() { - merge_with_modeline(settings.to_mut(), modeline); + merge_with_modeline(Arc::make_mut(&mut settings), modeline); } settings } - pub fn resolve<'a>( - buffer: Option<&'a Buffer>, + pub fn resolve( + buffer: Option<&Buffer>, override_language: Option<&LanguageName>, - cx: &'a App, - ) -> Cow<'a, LanguageSettings> { + cx: &App, + ) -> Arc { let Some(buffer) = buffer else { return AllLanguageSettings::get(None, cx).language(None, override_language, cx); }; + + if let Some(resolved) = buffer.resolved_settings() + && override_language.is_none_or(|language| { + Some(language) == buffer.language().map(|l| l.name()).as_ref() + }) + { + return resolved.clone(); + } + + Self::resolve_uncached(buffer, override_language, cx) + } + + pub(crate) fn resolve_uncached( + buffer: &Buffer, + override_language: Option<&LanguageName>, + cx: &App, + ) -> Arc { let location = buffer.file().map(|f| SettingsLocation { worktree_id: f.worktree_id(cx), path: f.path().as_ref(), }); - let all = AllLanguageSettings::get(location, cx); - let mut settings = if override_language.is_none() { - all.language(location, buffer.language().map(|l| l.name()).as_ref(), cx) - } else { - all.language(location, override_language, cx) - }; + let buffer_language = buffer.language().map(|l| l.name()); + let language_name = override_language.or(buffer_language.as_ref()); + let mut settings = + AllLanguageSettings::get(location, cx).language(location, language_name, cx); if let Some(modeline) = buffer.modeline() { - merge_with_modeline(settings.to_mut(), modeline); + merge_with_modeline(Arc::make_mut(&mut settings), modeline); } settings @@ -643,12 +664,12 @@ impl From for EditPredictionPromptFormat { impl AllLanguageSettings { /// Returns the [`LanguageSettings`] for the language with the specified name. - pub fn language<'a>( - &'a self, - location: Option>, + pub fn language( + &self, + location: Option>, language_name: Option<&LanguageName>, - cx: &'a App, - ) -> Cow<'a, LanguageSettings> { + cx: &App, + ) -> Arc { let settings = language_name .and_then(|name| self.languages.get(name)) .unwrap_or(&self.defaults); @@ -660,11 +681,11 @@ impl AllLanguageSettings { .properties(location.worktree_id, location.path) }); if let Some(editorconfig_properties) = editorconfig_properties { - let mut settings = settings.clone(); + let mut settings = (**settings).clone(); merge_with_editorconfig(&mut settings, &editorconfig_properties); - Cow::Owned(settings) + Arc::new(settings) } else { - Cow::Borrowed(settings) + Arc::clone(settings) } } @@ -873,7 +894,7 @@ impl settings::Settings for AllLanguageSettings { } } - let default_language_settings = load_from_content(all_languages.defaults.clone()); + let default_language_settings = Arc::new(load_from_content(all_languages.defaults.clone())); let mut languages = HashMap::default(); for (language_name, settings) in &all_languages.languages.0 { @@ -881,7 +902,7 @@ impl settings::Settings for AllLanguageSettings { settings::merge_from::MergeFrom::merge_from(&mut language_settings, settings); languages.insert( LanguageName(language_name.clone().into()), - load_from_content(language_settings), + Arc::new(load_from_content(language_settings)), ); } diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 4dae5ecb58..ab1f8c70e2 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -119,6 +119,7 @@ pub enum Event { Reloaded, CapabilityChanged, LanguageChanged(BufferId, bool), + SettingsChanged, Reparsed(BufferId), Saved, FileHandleChanged, @@ -2039,6 +2040,7 @@ impl MultiBuffer { BufferEvent::LanguageChanged(has_language) => { Event::LanguageChanged(buffer_id, *has_language) } + BufferEvent::SettingsChanged => Event::SettingsChanged, BufferEvent::Reparsed => Event::Reparsed(buffer_id), BufferEvent::DiagnosticsUpdated => Event::DiagnosticsUpdated, BufferEvent::CapabilityChanged => { @@ -2164,7 +2166,7 @@ impl MultiBuffer { .and_then(|(buffer, offset)| buffer.read(cx).language_at(offset)) } - pub fn language_settings<'a>(&'a self, cx: &'a App) -> Cow<'a, LanguageSettings> { + pub fn language_settings(&self, cx: &App) -> Arc { let snapshot = self.snapshot(cx); snapshot .excerpts @@ -2174,15 +2176,11 @@ impl MultiBuffer { .unwrap_or_else(move || self.language_settings_at(MultiBufferOffset::default(), cx)) } - pub fn language_settings_at<'a, T: ToOffset>( - &'a self, - point: T, - cx: &'a App, - ) -> Cow<'a, LanguageSettings> { + pub fn language_settings_at(&self, point: T, cx: &App) -> Arc { if let Some((buffer, offset)) = self.point_to_buffer_offset(point, cx) { LanguageSettings::for_buffer_at(buffer.read(cx), offset, cx) } else { - Cow::Borrowed(&AllLanguageSettings::get_global(cx).defaults) + AllLanguageSettings::get_global(cx).defaults.clone() } } @@ -6202,7 +6200,7 @@ impl MultiBufferSnapshot { .and_then(|(buffer, offset)| buffer.language_at(offset)) } - fn language_settings<'a>(&'a self, cx: &'a App) -> Cow<'a, LanguageSettings> { + fn language_settings(&self, cx: &App) -> Arc { self.excerpts .first() .map(|excerpt| excerpt.buffer_snapshot(self)) @@ -6210,15 +6208,11 @@ impl MultiBufferSnapshot { .unwrap_or_else(move || self.language_settings_at(MultiBufferOffset::ZERO, cx)) } - pub fn language_settings_at<'a, T: ToOffset>( - &'a self, - point: T, - cx: &'a App, - ) -> Cow<'a, LanguageSettings> { + pub fn language_settings_at(&self, point: T, cx: &App) -> Arc { if let Some((buffer, offset)) = self.point_to_buffer_offset(point) { buffer.settings_at(offset, cx) } else { - Cow::Borrowed(&AllLanguageSettings::get_global(cx).defaults) + AllLanguageSettings::get_global(cx).defaults.clone() } } diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 8bb16554ca..65e9fa9dbc 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -102,9 +102,14 @@ fn test_remote(cx: &mut App) { let ops = cx .foreground_executor() .block_on(host_buffer.read(cx).serialize_ops(None, cx)); - let mut buffer = - Buffer::from_proto(ReplicaId::REMOTE_SERVER, Capability::ReadWrite, state, None) - .unwrap(); + let mut buffer = Buffer::from_proto( + ReplicaId::REMOTE_SERVER, + Capability::ReadWrite, + state, + None, + cx, + ) + .unwrap(); buffer.apply_ops( ops.into_iter() .map(|op| language::proto::deserialize_operation(op).unwrap()), diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index 6f97482836..b9fb887d49 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -174,7 +174,7 @@ impl RemoteBufferStore { proto::create_buffer_for_peer::Variant::State(mut state) => { let buffer_id = BufferId::new(state.id)?; - let buffer_result = maybe!({ + let buffer_file_result = maybe!({ let mut buffer_file = None; if let Some(file) = state.file.take() { let worktree_id = worktree::WorktreeId::from_proto(file.worktree_id); @@ -188,12 +188,15 @@ impl RemoteBufferStore { buffer_file = Some(Arc::new(File::from_proto(file, worktree, cx)?) as Arc); } - Buffer::from_proto(replica_id, capability, state, buffer_file) + anyhow::Ok(buffer_file) }); - match buffer_result { - Ok(buffer) => { - let buffer = cx.new(|_| buffer); + match buffer_file_result { + Ok(buffer_file) => { + let buffer = cx.new(|cx| { + Buffer::from_proto(replica_id, capability, state, buffer_file, cx) + .expect("buffer_id was validated above") + }); self.loading_remote_buffers_by_id.insert(buffer_id, buffer); } Err(error) => { @@ -704,8 +707,9 @@ impl LocalBufferStore { ) }) .await; - cx.insert_entity(reservation, |_| { - let mut buffer = Buffer::build(text_buffer, Some(loaded.file), capability); + cx.insert_entity(reservation, |cx| { + let mut buffer = + Buffer::build(text_buffer, Some(loaded.file), capability, cx); buffer.set_encoding(loaded.encoding); buffer.set_has_bom(loaded.has_bom); buffer @@ -725,6 +729,7 @@ impl LocalBufferStore { is_private: false, })), Capability::ReadWrite, + cx, ); apply_initial_line_ending(&mut buffer, cx); buffer diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 9f88a70f29..bd4d53ab54 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1619,7 +1619,7 @@ impl LocalLspStore { .map(|(adapter, lsp)| (adapter.clone(), lsp.clone())) .collect::>() }; - let settings = LanguageSettings::for_buffer(buffer, cx).into_owned(); + let settings = LanguageSettings::for_buffer(buffer, cx); let request_timeout = ProjectSettings::get_global(cx) .global_lsp_settings .get_request_timeout(); @@ -4953,10 +4953,41 @@ impl LspStore { self.on_buffer_reloaded(buffer, cx); } + language::BufferEvent::SettingsChanged => { + self.on_buffer_settings_changed(&buffer, cx); + } + _ => {} } } + fn on_buffer_settings_changed(&mut self, buffer: &Entity, cx: &mut Context) { + let Some(prettier_store) = self.as_local().map(|s| s.prettier_store.clone()) else { + return; + }; + let buffer = buffer.read(cx); + if buffer.language().is_none() { + return; + } + let settings = LanguageSettings::for_buffer(buffer, cx); + if settings.prettier.allowed + && let Some(prettier_plugins) = prettier_store::prettier_plugins_for_language(&settings) + { + let worktree_id = File::from_dyn(buffer.file()).map(|file| file.worktree_id(cx)); + let prettier_plugins = prettier_plugins + .iter() + .map(|plugin| Arc::from(plugin.as_str())) + .collect::>(); + prettier_store.update(cx, |prettier_store, cx| { + prettier_store.install_default_prettier( + worktree_id, + prettier_plugins.into_iter(), + cx, + ) + }) + } + } + fn on_buffer_added(&mut self, buffer: &Entity, cx: &mut Context) -> Result<()> { buffer .read(cx) @@ -5334,7 +5365,7 @@ impl LspStore { log::debug!("Parsed modeline settings: {:?}", modeline_settings); - buffer_handle.update(cx, |buffer, _cx| buffer.set_modeline(modeline_settings)) + buffer_handle.update(cx, |buffer, cx| buffer.set_modeline(modeline_settings, cx)) } fn detect_language_for_buffer( @@ -5408,8 +5439,7 @@ impl LspStore { Some(&buffer_entity.read(cx)), Some(&new_language.name()), cx, - ) - .into_owned(); + ); let buffer_file = File::from_dyn(buffer_file.as_ref()); let worktree_id = if let Some(file) = buffer_file { @@ -5770,27 +5800,8 @@ impl LspStore { } fn on_settings_changed(&mut self, cx: &mut Context) { - let mut language_formatters_to_check = Vec::new(); - for buffer in self.buffer_store.read(cx).buffers() { - let buffer = buffer.read(cx); - let settings = LanguageSettings::for_buffer(buffer, cx); - if buffer.language().is_some() { - let buffer_file = File::from_dyn(buffer.file()); - language_formatters_to_check.push(( - buffer_file.map(|f| f.worktree_id(cx)), - settings.into_owned(), - )); - } - } - self.request_workspace_config_refresh(); - if let Some(prettier_store) = self.as_local().map(|s| s.prettier_store.clone()) { - prettier_store.update(cx, |prettier_store, cx| { - prettier_store.on_settings_changed(language_formatters_to_check, cx) - }) - } - let new_semantic_token_rules = crate::project_settings::ProjectSettings::get_global(cx) .global_lsp_settings .semantic_token_rules diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index 949b80802e..a5a32ff52b 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -696,31 +696,6 @@ impl PrettierStore { not_installed_plugins: plugins_to_install, }; } - - pub fn on_settings_changed( - &mut self, - language_formatters_to_check: Vec<(Option, LanguageSettings)>, - cx: &mut Context, - ) { - let mut prettier_plugins_by_worktree = HashMap::default(); - for (worktree, language_settings) in language_formatters_to_check { - if language_settings.prettier.allowed - && let Some(plugins) = prettier_plugins_for_language(&language_settings) - { - prettier_plugins_by_worktree - .entry(worktree) - .or_insert_with(HashSet::default) - .extend(plugins.iter().cloned()); - } - } - for (worktree, prettier_plugins) in prettier_plugins_by_worktree { - self.install_default_prettier( - worktree, - prettier_plugins.into_iter().map(Arc::from), - cx, - ); - } - } } pub fn prettier_plugins_for_language( diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 0818800e00..ad3a787fe1 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -326,14 +326,14 @@ async fn test_editorconfig_support(cx: &mut gpui::TestAppContext) { cx.executor().run_until_parked(); - let settings_for = async |path: &str, cx: &mut TestAppContext| -> LanguageSettings { + let settings_for = async |path: &str, cx: &mut TestAppContext| -> Arc { let buffer = project .update(cx, |project, cx| { project.open_buffer((worktree.read(cx).id(), rel_path(path)), cx) }) .await .unwrap(); - cx.update(|cx| LanguageSettings::for_buffer(&buffer.read(cx), cx).into_owned()) + cx.update(|cx| LanguageSettings::for_buffer(&buffer.read(cx), cx)) }; let settings_a = settings_for("a.rs", cx).await; @@ -411,14 +411,14 @@ async fn test_external_editorconfig_support(cx: &mut gpui::TestAppContext) { let worktree = project.update(cx, |project, cx| project.worktrees(cx).next().unwrap()); cx.executor().run_until_parked(); - let settings_for = async |path: &str, cx: &mut TestAppContext| -> LanguageSettings { + let settings_for = async |path: &str, cx: &mut TestAppContext| -> Arc { let buffer = project .update(cx, |project, cx| { project.open_buffer((worktree.read(cx).id(), rel_path(path)), cx) }) .await .unwrap(); - cx.update(|cx| LanguageSettings::for_buffer(&buffer.read(cx), cx).into_owned()) + cx.update(|cx| LanguageSettings::for_buffer(&buffer.read(cx), cx)) }; let settings_rs = settings_for("main.rs", cx).await; @@ -468,7 +468,7 @@ async fn test_internal_editorconfig_root_stops_traversal(cx: &mut gpui::TestAppC .await .unwrap(); cx.update(|cx| { - let settings = LanguageSettings::for_buffer(buffer.read(cx), cx).into_owned(); + let settings = LanguageSettings::for_buffer(buffer.read(cx), cx); assert_eq!(Some(settings.tab_size), NonZeroU32::new(2)); }); } @@ -757,7 +757,7 @@ async fn test_adding_worktree_discovers_external_editorconfigs(cx: &mut gpui::Te .unwrap(); cx.update(|cx| { - let settings = LanguageSettings::for_buffer(&buffer.read(cx), cx).into_owned(); + let settings = LanguageSettings::for_buffer(&buffer.read(cx), cx); // Test existing worktree has tab_size = 7 assert_eq!(Some(settings.tab_size), NonZeroU32::new(7)); diff --git a/crates/settings/src/editorconfig_store.rs b/crates/settings/src/editorconfig_store.rs index df858f89a1..d6b1a0ff1b 100644 --- a/crates/settings/src/editorconfig_store.rs +++ b/crates/settings/src/editorconfig_store.rs @@ -1,13 +1,18 @@ use anyhow::{Context as _, Result}; use collections::{BTreeMap, BTreeSet, HashSet}; -use ec4rs::{ConfigParser, PropertiesSource, Section}; +use ec4rs::{ + ConfigParser, PropertyKey as _, Section, + property::{ + EndOfLine, FinalNewline, IndentSize, IndentStyle, MaxLineLen, TabWidth, TrimTrailingWs, + }, +}; use fs::Fs; use futures::StreamExt; use gpui::{Context, EventEmitter, Task}; use paths::EDITORCONFIG_NAME; use smallvec::SmallVec; use std::{path::Path, str::FromStr, sync::Arc}; -use util::{ResultExt as _, rel_path::RelPath}; +use util::rel_path::RelPath; use crate::{InvalidSettingsError, LocalSettingsPath, WorktreeId, watch_config_file}; @@ -323,8 +328,14 @@ impl EditorconfigStore { for_worktree: WorktreeId, for_path: &RelPath, ) -> Option { - let mut properties = EditorconfigProperties::new(); let state = self.worktree_state.get(&for_worktree); + let has_internal_configs = state.is_some_and(|state| !state.internal_configs.is_empty()); + let has_external_configs = self.external_configs(for_worktree).next().is_some(); + if !has_internal_configs && !has_external_configs { + return None; + } + + let mut properties = EditorconfigProperties::new(); let internal_root_config_is_root = state .and_then(|state| state.internal_configs.get(RelPath::empty())) .and_then(|data| data.1.as_ref()) @@ -339,7 +350,7 @@ impl EditorconfigStore { properties = EditorconfigProperties::new(); } for section in &parsed_editorconfig.sections { - section.apply_to(&mut properties, std_path).log_err()?; + apply_relevant_properties(section, std_path, &mut properties); } } } @@ -363,7 +374,7 @@ impl EditorconfigStore { properties = EditorconfigProperties::new(); } for section in &config.sections { - section.apply_to(&mut properties, std_path).log_err()?; + apply_relevant_properties(section, std_path, &mut properties); } } } @@ -373,6 +384,30 @@ impl EditorconfigStore { } } +fn apply_relevant_properties( + section: &Section, + std_path: &Path, + properties: &mut EditorconfigProperties, +) { + if !section.applies_to(std_path) { + return; + } + let relevant_keys = [ + IndentStyle::key(), + IndentSize::key(), + TabWidth::key(), + EndOfLine::key(), + MaxLineLen::key(), + FinalNewline::key(), + TrimTrailingWs::key(), + ]; + for (key, value) in section.props().iter() { + if relevant_keys.contains(&key) { + properties.insert_raw_for_key(key, value.clone()); + } + } +} + #[cfg(any(test, feature = "test-support"))] impl EditorconfigStore { pub fn test_state(&self) -> (Vec, Vec>, Vec>) { @@ -393,3 +428,113 @@ impl EditorconfigStore { .unwrap_or_default() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_properties_resolution_and_updates() { + let mut store = EditorconfigStore::default(); + let worktree_id = WorktreeId::from_usize(1); + let root = LocalSettingsPath::InWorktree(Arc::from(RelPath::empty())); + let file_path = RelPath::from_unix_str("src/main.rs").unwrap(); + + assert_eq!(store.properties(worktree_id, file_path), None); + + store + .set_configs( + worktree_id, + root.clone(), + Some("root = true\n\n[*]\nindent_size = 4\n"), + ) + .unwrap(); + let properties = store.properties(worktree_id, file_path).unwrap(); + assert_eq!( + properties.get::().ok(), + Some(IndentSize::Value(4)) + ); + + store + .set_configs( + worktree_id, + root.clone(), + Some("root = true\n\n[*]\nindent_size = 8\n"), + ) + .unwrap(); + let properties = store.properties(worktree_id, file_path).unwrap(); + assert_eq!( + properties.get::().ok(), + Some(IndentSize::Value(8)) + ); + + store.set_configs(worktree_id, root, None).unwrap(); + assert_eq!(store.properties(worktree_id, file_path), None); + } + + #[test] + fn test_nested_config_overrides_and_irrelevant_keys() { + let mut store = EditorconfigStore::default(); + let worktree_id = WorktreeId::from_usize(1); + let file_path = RelPath::from_unix_str("src/main.rs").unwrap(); + + store + .set_configs( + worktree_id, + LocalSettingsPath::InWorktree(Arc::from(RelPath::empty())), + Some( + "root = true\n\n[*]\nindent_size = 4\nmax_line_length = 100\ncurly_bracket_next_line = true\n", + ), + ) + .unwrap(); + store + .set_configs( + worktree_id, + LocalSettingsPath::InWorktree(Arc::from(RelPath::from_unix_str("src").unwrap())), + Some("[*.rs]\nindent_size = 2\n"), + ) + .unwrap(); + + let properties = store.properties(worktree_id, file_path).unwrap(); + assert_eq!( + properties.get::().ok(), + Some(IndentSize::Value(2)) + ); + assert_eq!( + properties.get::().ok(), + Some(MaxLineLen::Value(100)) + ); + assert_eq!( + properties + .get_raw_for_key("curly_bracket_next_line") + .into_option(), + None + ); + + let other_file = RelPath::from_unix_str("src/main.js").unwrap(); + let properties = store.properties(worktree_id, other_file).unwrap(); + assert_eq!( + properties.get::().ok(), + Some(IndentSize::Value(4)) + ); + } + + #[test] + fn test_no_properties_after_worktree_removal() { + let mut store = EditorconfigStore::default(); + let worktree_id = WorktreeId::from_usize(1); + let file_path = RelPath::from_unix_str("src/main.rs").unwrap(); + + store + .set_configs( + worktree_id, + LocalSettingsPath::InWorktree(Arc::from(RelPath::empty())), + Some("root = true\n\n[*]\nindent_size = 4\n"), + ) + .unwrap(); + assert!(store.properties(worktree_id, file_path).is_some()); + + store.remove_for_worktree(worktree_id); + assert_eq!(store.properties(worktree_id, file_path), None); + } +}