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.

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

What kind of input does ignoreFiles expect? #11

Closed
ghost opened this issue Sep 2, 2015 · 8 comments
Closed

What kind of input does ignoreFiles expect? #11

ghost opened this issue Sep 2, 2015 · 8 comments

Comments

@ghost
Copy link

ghost commented Sep 2, 2015

It would make sense for it to accept an array of minimatch strings, but that does not seem to be the case. So what does it accept? I couldn't find it in the documentation.

@avioli
Copy link

avioli commented Sep 3, 2015

The sample on the overview page shows RegExp objects.

This is straight from the source code:

    function shouldIgnore(file) {
        return !_.some(options.ignoreFiles, function(ignore) {
            return ignore.test(file);
        });
    }

So ignoreFiles should be an array of RegExp objects, since .test() is a method on RegExp objects.

@ghost
Copy link
Author

ghost commented Sep 3, 2015

I should have known that. Thanks for the reply!

On 03-09-15 02:49, Evo Stamatov wrote:

The sample on the overview page shows RegExp objects.

This is straight from the source code:

 function  shouldIgnore(file) {
     return  !_.some(options.ignoreFiles,function(ignore) {
         return  ignore.test(file);
     });
 }

So |ignoreFiles| should be an array of RegExp objects, since |.test()|
is a method on RegExp objects
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test.


Reply to this email directly or view it on GitHub
#11 (comment).

@ghost
Copy link
Author

ghost commented Sep 3, 2015

By the way, I think it would be a lot simpler to switch to a filter that only process files that match it. Not the other way around, like it currently is. With the current ignoreFiles option I have to use this regex:

ignoreFiles: [/.*\.(js|css|jpg|git|md|htaccess|xml|txt)$/],

Whereas a filter option set to **/*.html by default would cover most, if not all, use cases.

@boushley
Copy link
Contributor

boushley commented Sep 3, 2015

@avioli Thanks for helping out and showing what needs to be done.

@ismay That's a great addition. You open to making a pull request that adds that as an option? And includeFiles option that does the opposite of ignoreFiles?

@ghost
Copy link
Author

ghost commented Sep 3, 2015

I don't know. I think the only way for such a new option to work is if it completely replaced the ignoreFiles option. I think it would be much better. With good defaults most people wouldn't even have to change the options.

But that would be a breaking change. I'd be up for submitting a pr, but I haven't had much luck with getting this plugin to work. If you could write some tests for the basic functionality I wouldn't mind taking a look at it.

@ghost ghost mentioned this issue Sep 5, 2015
@avioli
Copy link

avioli commented Sep 6, 2015

One solution, which would be quite easy, is to add onlyIncludeFiles array as well, negating the return value (fiddle: http://jsfiddle.net/sbyx2edh/), so the code should look like this (not tested other than the fiddle):

function plugin(options) {
    //...
    defaultsDeep(options, {
        ignoreFiles: [],
        onlyIncludeFiles: [/./], // will include all files by default
    //...

    function shouldInclude(file) {
        return _.some(options.onlyIncludeFiles, function(ignore) {
            return ignore.test(file);
        });
    }

An alternative is to use this RegExp: /(?=\.(html|htm))/ without modifying the plugin.

It uses Positive lookahead group construct to match without consuming characters, but has an issue that it cannot match strictly end of line, so /(?=\.(html|htm))$/ is invalid. I believe this shouldn't be a problem, given metalsmith is a static website generator, meaning you are in complete control of the naming conventions - don't name a file with .html in its name, or it will be included in the sitemap.

Here is a fiddle, you can play around and see what I mean with the .html in the name: https://jsfiddle.net/8fzbpw9f/

@avioli
Copy link

avioli commented Sep 6, 2015

I just saw the refactor, so sorry if this isn't helpful at all. I believe the plugin should remain as simple as possible, so whatever @boushley decides is fine by me.

@ghost
Copy link
Author

ghost commented Sep 6, 2015

@avioli np, simplicity is what I'm going for as well. That and a user friendly API.

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

No branches or pull requests

2 participants