Skip to content
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.

Sign up for GitHub

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

Jump to bottom

extensions: Select language extension based on longest matching path extension #11697

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

extensions: Select language extension based on longest matching path extension #11697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bf81e6
refactor(ts): remove unneeded extensions
claytonrcarter May 11, 2024
8c60680
feat(extensions): match language extensions based on longest matching…
claytonrcarter May 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 19 additions & 10 deletions crates/language/src/language_registry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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));
Copy link
Contributor Author

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. 😄

Copy link
Collaborator

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:

let matching_suffix_len = config
    .path_suffixes
    .iter()
    .filter(|suffix| {
        filename == suffix ||
            filename.strip_suffix(suffix).map_or(false, |prefix| prefix.ends_with('.'))
    })
    .map(|suffix| suffix.len())
    .max();

let content_matches = content.zip(config.first_line_pattern.as_ref()).map_or(
false,
|(content, pattern)| {
Expand All @@ -533,8 +540,10 @@ impl LanguageRegistry {
},
);
if path_matches_custom_suffix {
2
} else if path_matches_default_suffix || content_matches {
usize::MAX
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used MAX to ensure that user overrides always win.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Expand Down
2 changes: 1 addition & 1 deletion crates/languages/src/typescript/config.toml
View file
Open in desktop
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"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 cts and mts are already handled).

line_comments = ["// "]
autoclose_before = ";:.,=}])>"
brackets = [
Expand Down