From 6eb2ffe77a1192b15e7fe00ed7ba3dee1f74f19a Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Thu, 27 Feb 2025 15:29:32 -0300 Subject: [PATCH] Support absolute `disabled_globs` (#25755) Closes: #25556 We were always comparing `disabled_globs` against the relative file path, we'll now use the absolute path if the glob is also absolute. Release Notes: - Support absolute globs in `edit_predictions.disabled_globs` --- assets/settings/default.json | 2 + .../src/copilot_completion_provider.rs | 2 +- crates/editor/src/editor.rs | 2 +- crates/editor/src/items.rs | 1 + .../src/inline_completion_button.rs | 8 +- crates/language/src/buffer.rs | 24 ++- crates/language/src/buffer_tests.rs | 1 + crates/language/src/language_settings.rs | 167 +++++++++++++++++- 8 files changed, 191 insertions(+), 16 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 21d90f17dd..6cf64ffd52 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -829,6 +829,8 @@ // A list of globs representing files that edit predictions should be disabled for. // There's a sensible default list of globs already included. // Any addition to this list will be merged with the default list. + // Globs are matched relative to the worktree root, + // except when starting with a slash (/) or equivalent in Windows. "disabled_globs": [ "**/.env*", "**/*.pem", diff --git a/crates/copilot/src/copilot_completion_provider.rs b/crates/copilot/src/copilot_completion_provider.rs index e6757e9d7f..b6e854eacf 100644 --- a/crates/copilot/src/copilot_completion_provider.rs +++ b/crates/copilot/src/copilot_completion_provider.rs @@ -192,7 +192,7 @@ impl EditPredictionProvider for CopilotCompletionProvider { fn discard(&mut self, cx: &mut Context) { let settings = AllLanguageSettings::get_global(cx); - let copilot_enabled = settings.show_inline_completions(None, cx); + let copilot_enabled = settings.show_edit_predictions(None, cx); if !copilot_enabled { return; diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 09cbdcf8be..e5a0c926d3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5005,7 +5005,7 @@ impl Editor { return Some(true); }; let settings = all_language_settings(Some(file), cx); - Some(settings.inline_completions_enabled_for_path(file.path())) + Some(settings.edit_predictions_enabled_for_file(file, cx)) }) .unwrap_or(false) } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 24f94027ca..d3c5089af2 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1739,6 +1739,7 @@ mod tests { let file = TestFile { path: Path::new("").into(), root_name: String::new(), + local_root: None, }; assert_eq!(path_for_file(&file, 0, false, cx), None); } diff --git a/crates/inline_completion_button/src/inline_completion_button.rs b/crates/inline_completion_button/src/inline_completion_button.rs index 2ba279b8da..1a842bf979 100644 --- a/crates/inline_completion_button/src/inline_completion_button.rs +++ b/crates/inline_completion_button/src/inline_completion_button.rs @@ -456,7 +456,7 @@ impl InlineCompletionButton { } let settings = AllLanguageSettings::get_global(cx); - let globally_enabled = settings.show_inline_completions(None, cx); + let globally_enabled = settings.show_edit_predictions(None, cx); menu = menu.toggleable_entry("All Files", globally_enabled, IconPosition::Start, None, { let fs = fs.clone(); move |_, cx| toggle_inline_completions_globally(fs.clone(), cx) @@ -702,7 +702,7 @@ impl InlineCompletionButton { Some( file.map(|file| { all_language_settings(Some(file), cx) - .inline_completions_enabled_for_path(file.path()) + .edit_predictions_enabled_for_file(file, cx) }) .unwrap_or(true), ) @@ -825,7 +825,7 @@ async fn open_disabled_globs_setting_in_editor( } fn toggle_inline_completions_globally(fs: Arc, cx: &mut App) { - let show_edit_predictions = all_language_settings(None, cx).show_inline_completions(None, cx); + let show_edit_predictions = all_language_settings(None, cx).show_edit_predictions(None, cx); update_settings_file::(fs, cx, move |file, _| { file.defaults.show_edit_predictions = Some(!show_edit_predictions) }); @@ -845,7 +845,7 @@ fn toggle_show_inline_completions_for_language( cx: &mut App, ) { let show_edit_predictions = - all_language_settings(None, cx).show_inline_completions(Some(&language), cx); + all_language_settings(None, cx).show_edit_predictions(Some(&language), cx); update_settings_file::(fs, cx, move |file, _| { file.languages .entry(language.name()) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 2d946dae16..5d91b617e0 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -4477,6 +4477,7 @@ impl IndentSize { pub struct TestFile { pub path: Arc, pub root_name: String, + pub local_root: Option, } #[cfg(any(test, feature = "test-support"))] @@ -4490,7 +4491,11 @@ impl File for TestFile { } fn as_local(&self) -> Option<&dyn LocalFile> { - None + if self.local_root.is_some() { + Some(self) + } else { + None + } } fn disk_state(&self) -> DiskState { @@ -4518,6 +4523,23 @@ impl File for TestFile { } } +#[cfg(any(test, feature = "test-support"))] +impl LocalFile for TestFile { + fn abs_path(&self, _cx: &App) -> PathBuf { + PathBuf::from(self.local_root.as_ref().unwrap()) + .join(&self.root_name) + .join(self.path.as_ref()) + } + + fn load(&self, _cx: &App) -> Task> { + unimplemented!() + } + + fn load_bytes(&self, _cx: &App) -> Task>> { + unimplemented!() + } +} + pub(crate) fn contiguous_ranges( values: impl Iterator, max_len: usize, diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 9d12dfcf48..d7cd2659b3 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -254,6 +254,7 @@ fn file(path: &str) -> Arc { Arc::new(TestFile { path: Path::new(path).into(), root_name: "zed".into(), + local_root: None, }) } diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 3adb8a8156..5a27427d6e 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -231,13 +231,33 @@ pub struct EditPredictionSettings { /// A list of globs representing files that edit predictions should be disabled for. /// This list adds to a pre-existing, sensible default set of globs. /// Any additional ones you add are combined with them. - pub disabled_globs: Vec, + pub disabled_globs: Vec, /// Configures how edit predictions are displayed in the buffer. pub mode: EditPredictionsMode, /// Settings specific to GitHub Copilot. pub copilot: CopilotSettings, } +impl EditPredictionSettings { + /// Returns whether edit predictions are enabled for the given path. + pub fn enabled_for_file(&self, file: &Arc, cx: &App) -> bool { + !self.disabled_globs.iter().any(|glob| { + if glob.is_absolute { + file.as_local() + .map_or(false, |local| glob.matcher.is_match(local.abs_path(cx))) + } else { + glob.matcher.is_match(file.path()) + } + }) + } +} + +#[derive(Clone, Debug)] +pub struct DisabledGlob { + matcher: GlobMatcher, + is_absolute: bool, +} + /// The mode in which edit predictions should be displayed. #[derive(Copy, Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] @@ -965,16 +985,12 @@ impl AllLanguageSettings { } /// Returns whether edit predictions are enabled for the given path. - pub fn inline_completions_enabled_for_path(&self, path: &Path) -> bool { - !self - .edit_predictions - .disabled_globs - .iter() - .any(|glob| glob.is_match(path)) + pub fn edit_predictions_enabled_for_file(&self, file: &Arc, cx: &App) -> bool { + self.edit_predictions.enabled_for_file(file, cx) } /// Returns whether edit predictions are enabled for the given language and path. - pub fn show_inline_completions(&self, language: Option<&Arc>, cx: &App) -> bool { + pub fn show_edit_predictions(&self, language: Option<&Arc>, cx: &App) -> bool { self.language(None, language.map(|l| l.name()).as_ref(), cx) .show_edit_predictions } @@ -1199,7 +1215,12 @@ impl settings::Settings for AllLanguageSettings { }, disabled_globs: completion_globs .iter() - .filter_map(|g| Some(globset::Glob::new(g).ok()?.compile_matcher())) + .filter_map(|g| { + Some(DisabledGlob { + matcher: globset::Glob::new(g).ok()?.compile_matcher(), + is_absolute: Path::new(g).is_absolute(), + }) + }) .collect(), mode: edit_predictions_mode, copilot: copilot_settings, @@ -1357,6 +1378,8 @@ pub struct PrettierSettings { #[cfg(test)] mod tests { + use gpui::TestAppContext; + use super::*; #[test] @@ -1401,6 +1424,132 @@ mod tests { assert!(result.is_err()); } + #[gpui::test] + fn test_edit_predictions_enabled_for_file(cx: &mut TestAppContext) { + use crate::TestFile; + use std::path::PathBuf; + + let cx = cx.app.borrow_mut(); + + let build_settings = |globs: &[&str]| -> EditPredictionSettings { + EditPredictionSettings { + disabled_globs: globs + .iter() + .map(|glob_str| { + #[cfg(windows)] + let glob_str = { + let mut g = String::new(); + + if glob_str.starts_with('/') { + g.push_str("C:"); + } + + g.push_str(&glob_str.replace('/', "\\")); + g + }; + #[cfg(windows)] + let glob_str = glob_str.as_str(); + + DisabledGlob { + matcher: globset::Glob::new(glob_str).unwrap().compile_matcher(), + is_absolute: Path::new(glob_str).is_absolute(), + } + }) + .collect(), + ..Default::default() + } + }; + + const WORKTREE_NAME: &str = "project"; + let make_test_file = |segments: &[&str]| -> Arc { + let mut path_buf = PathBuf::new(); + path_buf.extend(segments); + + Arc::new(TestFile { + path: path_buf.as_path().into(), + root_name: WORKTREE_NAME.to_string(), + local_root: Some(PathBuf::from(if cfg!(windows) { + "C:\\absolute\\" + } else { + "/absolute/" + })), + }) + }; + + let test_file = make_test_file(&["src", "test", "file.rs"]); + + // Test relative globs + let settings = build_settings(&["*.rs"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["*.txt"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test absolute globs + let settings = build_settings(&["/absolute/**/*.rs"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["/other/**/*.rs"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test exact path match relative + let settings = build_settings(&["src/test/file.rs"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["src/test/otherfile.rs"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test exact path match absolute + let settings = build_settings(&[&format!("/absolute/{}/src/test/file.rs", WORKTREE_NAME)]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["/other/test/otherfile.rs"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test * glob + let settings = build_settings(&["*"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["*.txt"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test **/* glob + let settings = build_settings(&["**/*"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["other/**/*"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test directory/** glob + let settings = build_settings(&["src/**"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + + let test_file_root: Arc = Arc::new(TestFile { + path: PathBuf::from("file.rs").as_path().into(), + root_name: WORKTREE_NAME.to_string(), + local_root: Some(PathBuf::from("/absolute/")), + }); + assert!(settings.enabled_for_file(&test_file_root, &cx)); + + let settings = build_settings(&["other/**"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test **/directory/* glob + let settings = build_settings(&["**/test/*"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["**/other/*"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test multiple globs + let settings = build_settings(&["*.rs", "*.txt", "src/**"]); + assert!(!settings.enabled_for_file(&test_file, &cx)); + let settings = build_settings(&["*.txt", "*.md", "other/**"]); + assert!(settings.enabled_for_file(&test_file, &cx)); + + // Test dot files + let dot_file = make_test_file(&[".config", "settings.json"]); + let settings = build_settings(&[".*/**"]); + assert!(!settings.enabled_for_file(&dot_file, &cx)); + + let dot_env_file = make_test_file(&[".env"]); + let settings = build_settings(&[".env"]); + assert!(!settings.enabled_for_file(&dot_env_file, &cx)); + } + #[test] pub fn test_resolve_language_servers() { fn language_server_names(names: &[&str]) -> Vec {