Skip to content
This repository has been archived by the owner on Jan 25, 2020. It is now read-only.

Add ignore support #75

Closed
wants to merge 3 commits into from

Conversation

lmarkus
Copy link
Contributor

@lmarkus lmarkus commented Jan 2, 2016

Adds a configurable list of files to ignore.
Uses minimatch to provide shell glob patterns, similar to .*ignore files.

Basically, instead of simply specifying the path to the directory, you can now optionally provide an array like:

{
    directory: {
        path: 'path/to/controllers',
        ignore: [
            'noExtensionFile',              // It will ignore any file named "noExtensionFile".
            '**/_CVS/*',                    // It will ignore the contents of any "_CSV" directory.
            '/subignore/notAController.js'  // It will ignore this specific file, relative to the path above.
        ]
    }
}

Another proposal to fix #73

If this looks OK, I'll write in the documentation updates before it's merged.

lmarkus added 3 commits January 1, 2016 22:22
This commit simply adds a few files that should be ignored( eg: files with no extension, files that look like they meet the API but don't)
to the "ignore" fixtures directory.
These files a perfectly valid use case. For example, SVN-like versioning systems populate a project with
hidden folders that include such files. The pattern is: A non-empty file without an extension.

These types of files will currently cause express enrouten to fail, as it will incorrectly identify them as
a valid node module, and will try to load them, as well as feed them the router object.
Fixes krakenjs#73

This introduces a change to the directory configuration (It's now an {path:"path/to/controllers", ignore:['pattern1',...]} object instead of a path string.
However, I made it backwards compatible. It shows a deprecation message, but we could support both methods if desired

// Cast legacy config into new config.
if (typeof options.directory === 'string') {
console.error('express-enrouten:\n' +
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to print this? Why not simply support a string as short hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's completely optional.
No idea if you guys want to support both methods of config. I can remove as needed.

Copy link
Member

Choose a reason for hiding this comment

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

K, I will leave it up to whoever merges / doesn't merge.

@tlivings
Copy link
Member

Aside from comment above, LGTM.

@jasisk jasisk closed this Jan 29, 2016
@jasisk
Copy link
Member

jasisk commented Jan 29, 2016

Closed due to branch renaming. Will reconstruct, reopen, and notify you when complete.

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.

None yet

3 participants