Repository: helix-editor/helix
File: helix-core/src/syntax.rs (Rust)

Code before:
        self.matcher
            .matches(path)
            .iter()
            .filter_map(|idx| self.file_types.get(*idx))
            .max_by_key(|file_type| file_type.glob.glob().len())
            .map(|file_type| &file_type.language_id)
    }
}

// Expose loader as Lazy<> global since it's always static?

#[derive(Debug)]
pub struct Loader {
    // highlight_names ?
    language_configs: Vec<Arc<LanguageConfiguration>>,
    language_config_ids_by_extension: HashMap<String, usize>, // Vec<usize>
    language_config_ids_glob_matcher: FileTypeGlobMatcher,
    language_config_ids_by_shebang: HashMap<String, usize>,

    language_server_configs: HashMap<String, LanguageServerConfiguration>,

    scopes: ArcSwap<Vec<String>>,
}

impl Loader {
    pub fn new(config: Configuration) -> Result<Self, globset::Error> {
        let mut language_configs = Vec::new();
        let mut language_config_ids_by_extension = HashMap::new();
        let mut language_config_ids_by_shebang = HashMap::new();
        let mut file_type_globs = Vec::new();

        for config in config.language {
            // get the next id
            let language_id = language_configs.len();

            for file_type in &config.file_types {
                // entry().or_insert(Vec::new).push(language_id);
                match file_type {
                    FileType::Extension(extension) => {
                        language_config_ids_by_extension.insert(extension.clone(), language_id);
                    }
                    FileType::Glob(glob) => {
                        file_type_globs.push(FileTypeGlob::new(glob.to_owned(), language_id));
                    }
                };
            }
            for shebang in &config.shebangs {
                language_config_ids_by_shebang.insert(shebang.clone(), language_id);
            }

            language_configs.push(Arc::new(config));

Review comment (bug):
Switching this to return a Result makes the error handling for config (re)loading a little bit messier. I think it's revealing an odd separation between `Configuration` and `Loader` that makes error handling awkward, and it might be a nice chance for a refactor.

I pushed up some ideas in this commit: https://github.com/the-mikedavis/helix/commit/6c455ec4c4b250c8170d632c6a3436b51c0e8f85

The main thing I want to avoid is handling the `Loader::new` error in `Application::new`. If either the Configuration and Loader creation fails we should report on it in this part of `main`: https://github.com/helix-editor/helix/blob/bbcc89241fd63b460ea7b1661a550f5b9eacba4c/helix-term/src/main.rs#L148-L155

To do that we basically handle `Loader` everywhere other than `helix_core` (or the health-check part of `xtask`) rather than `Configuration`, which I think is cleaner anyways.

Code after:
    fn new(file_types: Vec<FileTypeGlob>) -> Result<Self, globset::Error> {
        let mut builder = globset::GlobSetBuilder::new();
        for file_type in &file_types {
            builder.add(file_type.glob.clone());
        }

        Ok(Self {
            matcher: builder.build()?,
            file_types,
        })
    }

    fn language_id_for_path(&self, path: &Path) -> Option<&usize> {
        self.matcher
            .matches(path)
            .iter()
            .filter_map(|idx| self.file_types.get(*idx))
            .max_by_key(|file_type| file_type.glob.glob().len())
            .map(|file_type| &file_type.language_id)
    }
}

// Expose loader as Lazy<> global since it's always static?

#[derive(Debug)]
pub struct Loader {
    // highlight_names ?
    language_configs: Vec<Arc<LanguageConfiguration>>,
    language_config_ids_by_extension: HashMap<String, usize>, // Vec<usize>
    language_config_ids_glob_matcher: FileTypeGlobMatcher,
    language_config_ids_by_shebang: HashMap<String, usize>,

    language_server_configs: HashMap<String, LanguageServerConfiguration>,

    scopes: ArcSwap<Vec<String>>,
}

pub type LoaderError = globset::Error;

impl Loader {
    pub fn new(config: Configuration) -> Result<Self, LoaderError> {
        let mut language_configs = Vec::new();
        let mut language_config_ids_by_extension = HashMap::new();
        let mut language_config_ids_by_shebang = HashMap::new();
        let mut file_type_globs = Vec::new();

        for config in config.language {
            // get the next id
            let language_id = language_configs.len();

            for file_type in &config.file_types {