Skip to content
This repository was archived by the owner on Aug 20, 2020. It is now read-only.

Filter out hidden and extensionless files from watching #3

Merged

Conversation

vipentti
Copy link
Contributor

@vipentti vipentti commented Mar 5, 2019

Relates to discussion in rust-lang/rust-analyzer#869

I'm not sure if this is the appropriate place to do the filtering.

@matklad
Copy link
Member

matklad commented Mar 5, 2019

Yeah, I think we probably want to put the logic here to Root:

ra_vfs/src/roots.rs

Lines 72 to 102 in 31eec52

impl RootData {
fn new(path: PathBuf, excluded_dirs: Vec<RelativePathBuf>) -> RootData {
let mut canonical_path = path.canonicalize().ok();
if Some(&path) == canonical_path.as_ref() {
canonical_path = None;
}
RootData { path, canonical_path, excluded_dirs }
}
fn is_excluded(&self, path: &RelativePath) -> bool {
if self.excluded_dirs.iter().any(|it| it == path) {
return true;
}
// Ignore some common directories.
//
// FIXME: don't hard-code, specify at source-root creation time using
// gitignore
for (i, c) in path.components().enumerate() {
if let relative_path::Component::Normal(c) = c {
if (i == 0 && c == "target") || c == ".git" || c == "node_modules" {
return true;
}
}
}
match path.extension() {
None | Some("rs") => false,
_ => true,
}
}
}

Root is effectively a predicate which checks if a particular file belongs to it. The existing is_excluded function can't be used here: it purpusfully has access only to RelativePath, and so it can't do a syscall required to figure out if the path is file or directory. I think it's fine to add is_excluded_dir_entry alongside the is_excluded, which would do IO.

@vipentti
Copy link
Contributor Author

vipentti commented Mar 5, 2019

Where should the is_excluded_dir_entry be called from ? Roots::contains ? Do we have to worry about the IO performance or should it not matter since the files are only loaded once and then watched for changes ?

@vipentti
Copy link
Contributor Author

vipentti commented Mar 5, 2019

Couldn't we also run the check in is_excluded ?

fn is_excluded(&self, path: &RelativePath) -> bool {
    ///...

    let full_path = path.to_path(&self.path);

    /// use full_path for checking via metadata
}

In the case where we have a file without extension

@matklad
Copy link
Member

matklad commented Mar 5, 2019

@vipentti oh, right... Yeah, I think that'll work fine. I worry a tiny bit about making syscall there, from performance and purity point of view, but I guess we can always optimize later.

@vipentti vipentti force-pushed the filter-extensionless-and-hidden-files branch from 31eec52 to 55f333d Compare March 5, 2019 14:31
@bjorn3
Copy link

bjorn3 commented Mar 5, 2019

What about #[path=".abc.rs"] mod abc; or include!("abc.my_funky_ext");. I believe those will be incorrectly ignored.

@matklad
Copy link
Member

matklad commented Mar 5, 2019

@bjorn3 this is an intended limitation at the moment. A long term plan would be to allow the user to explicitly whitelist weird files for inclusion.

@bjorn3
Copy link

bjorn3 commented Mar 5, 2019

Got it.

@vipentti
Copy link
Contributor Author

vipentti commented Mar 5, 2019

I think .abc.rs should work if path.extension() reports its extension as rs, we filter only extension-less or hidden files after checking if they are rust files

        match path.extension() {
            Some("rs") => false,
            Some(_) => true,
            // Exclude extension-less and hidden files
            None => is_extensionless_or_hidden_file(&self.path, path),
        }

@matklad
Copy link
Member

matklad commented Mar 6, 2019

bors r+

bors bot added a commit that referenced this pull request Mar 6, 2019
3: Filter out hidden and extensionless files from watching r=matklad a=vipentti

Relates to discussion in rust-lang/rust-analyzer#869

I'm not sure if this is the appropriate place to do the filtering.

Co-authored-by: Ville Penttinen <[email protected]>
@bors
Copy link
Contributor

bors bot commented Mar 6, 2019

Build succeeded

  • rust-analyzer.ra_vfs

@bors bors bot merged commit 55f333d into rust-analyzer:master Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants