Exclude directories

[cedaro]

Exclude directories

[bradyvercher]

I considered various ways to to do this early on and had a few false starts before scrapping them. I figured I'd wait to see if there was any interest before diving down that rabbit hole. It would be nice to prevent the PHP tools from processing the node_modules and other dev-related directories without having to configure a build process, too.

These are the two viable approaches I came up with (although there are probably some I'm not thinking of):

Update the WP i18n Tools

The method in the PHP tools for extracting strings, MakePOT::xgettext(), has an optional parameter for accepting a list of files to exclude as an array. I'm not sure what the globbing pattern is, but it doesn't match Grunt and passing it as a CLI argument would probably require the list to be comma-separated. So:

1. The CLI interface would need to be updated to accept an additional argument
2. The plugin and theme methods would need to be modified to accept that argument
3. Those methods would then pass the excludes on to MakePOT::xgettext()
4. If there were "Template Name" headers in an excluded file/directory, the theme method would need to be updated to exclude those as well.

The extra CLI argument for excludes might make it incompatible with non-bundled tools (I'm not sure what would happen if it were passed to the default tools).

Node-based Solution

The other option I considered since this is primarily a build process, is to copy the project to a temporary directory, excluding any configured files, and run the makepot task there, then clean up the temporary directory afterward.

[tollmanz]

Hey @bradyvercher! Thanks for considering this. I will say that there is a real use case for this and I do think it would be worth the inclusion.

Update the WP i18n Tools

I'm not sure what the globbing pattern is, but it doesn't match Grunt

Not sure that this is a major problem. Since this is an option for a CLI underlying a Grunt module, I think it is fine to pass this as an option and not the typical Grunt file pattern.

I think that if MakePOT::xgettext() supports the exclusion, there is no reason that the CLI should not support it. While a lot of work, it may make sense to patch this and push it upstream and thus the tools won't be badly forked. We could even make the changes here and test it before pushing upstream.

Node-based Solution

I think this is the easiest solution and there are existing node modules that make this fairly easy (famous last words). An alternative approach would be to remove strings from the .pot file after it is created. Since each string is prefixed with a comment pointing to the string location, this could be used to remove those translations from the file. This is likely to be a little less stable, but could be a solution.

[bradyvercher]

Thanks for the feedback, @tollmanz. I agree that excluding directories is useful, I just wasn't ready to jump into it before. Modifying the PHP tools seemed a little easier and less error-prone since it basically just requires adding some new arguments and parameters, so I went that route for now.

To test this out (I've only tested with a couple of plugins so far), change the package version in package.json to the following URL and run npm install: git://github.com/blazersix/grunt-wp-i18n.git#develop

The globbing pattern turns out to be nothing more than a regex pattern passed to preg_match().

Add a new argument to the options object with a key of exclude and the value should be an array of regex patterns:

options: {
    exclude: ['subdir/.*']
}

Files in node_modules will be excluded automatically.

Like I mentioned in my first comment, when processing a theme, "Template Name" headers in excluded files will still be included, so it'd take a little extra work to handle those.

A good thing about this approach is that since it is a target option, a future node-based solution could still take advantage of all the benefits of Grunt files objects if needed.

[tollmanz]

@bradyvercher Nice work! I will try this out on Monday and let you know how it works with a theme. I have a specific scenario where I want a submodule to be exclude from the .pot file. It should be a good test.

Thanks, once again, for your awesome work!

[tollmanz]

@bradyvercher I am not able to get this working in a theme. I've tried the following:

{ exclude: ['src/inc/**/*'] }
{ exclude: ['src/inc/**/*.php'] }
{ exclude: ['src/inc/.*'] }

Nothing seems to work. With src/inc/**/*, I get:

Warning: preg_match(): Compilation failed: nothing to repeat at offset 16 in ../node_modules/grunt-wp-i18n/vendor/wp-i18n-tools/extract.php on line 59

[bradyvercher]

@tollmanz Hmm. I ran a quick test with just a style.css and a file with some gettext calls in an excluded directory and it seems to work. I don't think ** is valid regex, which is why that particular pattern throws a notice, but src/inc/.* should be fine. This is the basic Gruntfile config I tested with:

makepot: {
    target: {
        options: {
            exclude: ['src/inc/.*'],
            type: 'wp-theme'
        }
    }
}

You wouldn't be getting an error if it weren't the dev version of this package, so we can rule that out. Are you sure the POT file is actually being regenerated and you're not viewing an older version?

Do the strings from the excluded files exist elsewhere in your theme or are you still seeing references to the excluded files in the POT file?

[tollmanz]

@bradyvercher I got it working. Just some stupidity on my part.

My config looked like this:

makepot: {
    theme: {
        options: {
            cwd: 'src',
            mainFile: 'src/style.css',
            potFilename: 'theme.pot',
            type: 'wp-theme',
            exclude: [
                'src/inc/.*'
            ]
        }
    }
}

Because I had cwd set, I was pointing to the wrong directory in the exclude directive. Updating to the following allowed the exclude rules to work perfectly:

makepot: {
    theme: {
        options: {
            cwd: 'src',
            mainFile: 'src/style.css',
            potFilename: 'theme.pot',
            type: 'wp-theme',
            exclude: [
                'inc/.*'
            ]
        }
    }
}

Sorry for the false negative. This is looking great to me. Thanks as always!

[bradyvercher]

Glad to see it's working! I'm looking to remove the need to add the cwd to the mainFile option like that, but I think implementing some tests is probably the first order of business.

[bradyvercher]

@tollmanz Actually, I was able to get those tests and that change done a bit quicker than I thought, so with the latest dev version, the mainFile should be relative to cwd. You can really get rid of the mainFile option for themes entirely, though, since style.css is the default. I'll bump the version to 0.4.0 for the next release since that's a breaking change.

[tollmanz]

Sweet! Thanks for the head's up! Are you planning on bumping the version today?

[bradyvercher]

I'd like to, but hope to hear back on the bbPress issue first. Would you mind running npm install grunt-wp-i18n again and testing with the updates? You might have to run npm cache clean first if it doesn't pull down the latest commits.

[tollmanz]

All seems to work after that update! I've updated my config to:

makepot: {
    theme: {
        options: {
            cwd: 'src',
            potFilename: 'theme.pot',
            type: 'wp-theme',
            exclude: [
                'inc/.*'
            ]
        }
    }
}

[bradyvercher]

Awesome, thanks!

[philiparthurmoore]

Hey gang. I've been running grunt makepot with the following config:

        /**
         * Theme and Plugin internationalization
         * @see https://github.com/blazersix/grunt-wp-i18n/
         */
        makepot: {
            target: {
                options: {
                    domainPath: '/languages',
                    exclude: [
                        'node_modules/.*'
                    ],
                    i18nToolsPath: 'languages/tools/i18n',
                    potFilename: '<%= pkg.name %>-en_US.pot',
                    type: 'wp-theme'
                }
            }
        },

There are a few issues here:

node_modules exclusion isn't working. I know that this was added automatically so it's not needed, but even then node_modules stuff shows up:

#: node_modules/grunt-wp-i18n/test/fixtures/text-domains/update-all-domains.php:6
#: node_modules/grunt-wp-i18n/test/fixtures/text-domains/update-all-domains.php:7
#: node_modules/grunt-wp-i18n/test/fixtures/text-domains/update-all-domains.php:8
msgctxt "a string"
msgid "String"
msgstr ""

I suggest adding information into the documentation about i18nToolsPath so that people know they may need to create symbolic links to get that working. Right now I'm getting this output:

PHP Warning:  require_once(/Applications/XAMPP/xamppfiles/htdocs/develop/src/wp-content/themes/theme-slug/languages/src/wp-includes/pomo/po.php): failed to open stream: No such file or directory in /Applications/XAMPP/xamppfiles/htdocs/develop/src/wp-content/themes/theme-slug/languages/tools/i18n/not-gettexted.php on line 16

I want those tools to be tucked away under /languages/ and not rely on the trunk checkout, and it seems that I'm going to have to use symbolic links to make that work.

[shivapoudel]

@bradyvercher I am also uncleared about i18nToolsPath, could you please document for it as suggested by @philiparthurmoore xD

[bradyvercher]

Hi @philiparthurmoore! The i18nToolsPath should really only be defined if you want to use a different version of the i18n tools. It's mainly there as a relic from the initial version before the tools were bundled and adds a bit of flexibility for projects that need it.

Otherwise, the bundled tools are used, which have been customized to support the various options, including exclude, which is probably why your local copy isn't ignoring those.

Does everything work if you remove that i18nToolsPath option?

[philiparthurmoore]

Wow. And magically all of the above issues went away when I removed i18nToolsPath. Thanks so much @bradyvercher. I lost half a day before posting here; I should post more often.

[bradyvercher]

Ouch! Glad to hear that fixed it. Definitely feel free to suggest any improvements and reach out much sooner next time!

[GaryJones]

Maybe add a PR into the README that clarifies this so others don't get tripped up too?