Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Filter out dotfiles in code hints #6068

Merged
merged 3 commits into from
Nov 20, 2013
Merged

Conversation

dangoor
Copy link
Contributor

@dangoor dangoor commented Nov 20, 2013

Quick fix for #6067, but this may not be the total fix if we want to filter out more than dotfiles

@@ -908,7 +908,6 @@ define(function (require, exports, module) {
}
} else {
return !isDirectoryExcluded(entry.fullPath) &&
entry.name.indexOf(".") !== 0 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

This part of the patch seems to make one of the tests weaker. Was that intentional?

@iwehrman
Copy link
Contributor

I've pushed another commit to this branch that implements a fix discussed with @gruehle:

  1. Restore the directory dot-file test in addAllFilesAndSubdirectories;
  2. Hoist the JavaScript-language test out of addAllFilesAndSubdirectories and into isFileExcluded so that it also applies to the initial set of files sent to Tern in doEditorChange.

Over to @gruehle for review.

@ghost ghost assigned gruehle Nov 20, 2013
@@ -206,8 +206,13 @@ define(function (require, exports, module) {
if (!excludes) {
Copy link
Member

Choose a reason for hiding this comment

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

If excludes is null in the prefs, we won't exclude any file. I think this if statement needs to be moved into the code below so we will always do the dotfile and language checks.

@iwehrman
Copy link
Contributor

Fixed.

@gruehle
Copy link
Member

gruehle commented Nov 20, 2013

I verified that this fixes the hang/crash on Windows and all unit tests pass. I also did some light scenario testing with code hints and everything worked as expected.

Merging

gruehle added a commit that referenced this pull request Nov 20, 2013
@gruehle gruehle merged commit a1addee into master Nov 20, 2013
@gruehle gruehle deleted the dangoor/6067-reading-all-files branch November 20, 2013 23:18
@dangoor
Copy link
Contributor Author

dangoor commented Nov 21, 2013

Awesome. Thanks for the quick fixing and review @iwehrman, @gruehle and @peterflynn

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants