-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
extensions: Select language extension based on longest matching path extension #11697
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ use sum_tree::Bias; | |
use text::{Point, Rope}; | ||
use theme::Theme; | ||
use unicase::UniCase; | ||
use util::{maybe, paths::PathExt, post_inc, ResultExt}; | ||
use util::{maybe, post_inc, ResultExt}; | ||
|
||
pub struct LanguageRegistry { | ||
state: RwLock<LanguageRegistryState>, | ||
|
@@ -509,20 +509,27 @@ impl LanguageRegistry { | |
user_file_types: Option<&HashMap<Arc<str>, Vec<String>>>, | ||
) -> impl Future<Output = Result<Arc<Language>>> { | ||
let filename = path.file_name().and_then(|name| name.to_str()); | ||
let extension = path.extension_or_hidden_file_name(); | ||
let path_suffixes = [extension, filename]; | ||
let empty = Vec::new(); | ||
|
||
let rx = self.get_or_load_language(move |language_name, config| { | ||
let path_matches_default_suffix = config | ||
.path_suffixes | ||
.iter() | ||
.any(|suffix| path_suffixes.contains(&Some(suffix.as_str()))); | ||
let filename = match filename { | ||
Some(filename) => filename, | ||
None => return 0, | ||
}; | ||
let matching_default_suffix = config.path_suffixes.iter().fold(None, |acc, suffix| { | ||
let ext = ".".to_string() + suffix; | ||
let is_matched = filename.ends_with(&ext) || filename == suffix; | ||
match (is_matched, &acc) { | ||
(true, None) => Some(suffix), | ||
(true, Some(s)) if suffix.len() > s.len() => Some(suffix), | ||
(true, Some(_)) | (false, Some(_)) | (false, None) => acc, | ||
} | ||
}); | ||
let path_matches_custom_suffix = user_file_types | ||
.and_then(|types| types.get(language_name)) | ||
.unwrap_or(&empty) | ||
.iter() | ||
.any(|suffix| path_suffixes.contains(&Some(suffix.as_str()))); | ||
.any(|suffix| filename.ends_with(suffix)); | ||
let content_matches = content.zip(config.first_line_pattern.as_ref()).map_or( | ||
false, | ||
|(content, pattern)| { | ||
|
@@ -533,8 +540,10 @@ impl LanguageRegistry { | |
}, | ||
); | ||
if path_matches_custom_suffix { | ||
2 | ||
} else if path_matches_default_suffix || content_matches { | ||
usize::MAX | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the user override needs to always win. I think we could use the longest match, but in the event of a tie, favor the user-defined path suffix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One way to implement that is to multiple the longest matching suffix's length by two and then add one if it's a user-defined suffix. Curious what you think of that. |
||
} else if let Some(matched_suffix) = matching_default_suffix { | ||
matched_suffix.len() | ||
} else if content_matches { | ||
1 | ||
} else { | ||
0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
name = "TypeScript" | ||
grammar = "typescript" | ||
path_suffixes = ["ts", "cts", "d.cts", "d.mts", "mts"] | ||
path_suffixes = ["ts", "cts", "mts"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is not important to this PR – and to be honest, I didn't actually test this – but I noticed while working on this that these suffixes a. would never work (because of the issue this PR is trying to fix) and b. are redundant (because |
||
line_comments = ["// "] | ||
autoclose_before = ";:.,=}])>" | ||
brackets = [ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I forgot to update this like I did above to support the full file name and prevent (eg)
rs
also matching.cars
. I'll leave this as is for now, pending input from someone else on how to proceed. 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we should use the same logic for finding the suffix length, for both the default path suffixes and the user-defined path suffixes.
I'd write it something like this: