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

Add ability to switch textdomains #22

Closed
nathanrice opened this issue Jun 28, 2014 · 7 comments
Closed

Add ability to switch textdomains #22

nathanrice opened this issue Jun 28, 2014 · 7 comments

Comments

@nathanrice
Copy link

I ran into this recently where a project was changing names (this can happen for many reasons, but I would imagine the most common is when doing an official fork).

So, I needed to change all the references to 'original-textdomain' to 'new-textdomain'. But as far as I could tell, if a string already has a textomain, it gets ignored, even if the textdomain it has doesn't match the one in package.json.

So, I had to do a manual search and replace to get all the strings switched to the new textdomain.

Is there any way to have this grunt tool do a replacement, either with a new task, or make it the default behavior of addtextdomain?

@bradyvercher
Copy link
Member

The addtextdomain tool isn't very sophisticated, so it does have some limitations. As far as I'm aware, it will actually append the text domain if there's already an existing, different text domain, so it may not even work as well as a search/replace in this scenario.

The best bet may be to allow one or more old text domains to be specified as an option that should be updated. Something like:

addtextdomain: {
    options: {
        textdomain: 'newdomain',
        updateDomains: [ 'olddomain1', 'olddomain2' ]
    }
}

@remkus
Copy link

remkus commented Jun 30, 2014

As far as I'm aware, it will actually append the text domain if there's already an existing, different text domain

That's what it does, yes.

@GaryJones
Copy link
Contributor

Looks like https://github.com/stephenharris/grunt-checktextdomain has a feature than can correct (not just append) text domains.

@bradyvercher
Copy link
Member

Aside from the stuff necessary for passing the parameters from Grunt to PHP, this is really all that was needed to get this working.

Anyone up for testing it out or have any feedback on its usefulness?

@GaryJones
Copy link
Contributor

"grunt-wp-i18n": "git://github.com/blazersix/grunt-wp-i18n.git#develop",

^^^ in the package.json, and then cd into the project directory and run npm install will pull down the develop branch for testing.

Replace with:

"grunt-wp-i18n": "^0.4.6",

when finished.

I think the most obvious usefulness is when creating a new theme out of a starter theme.

The current updating works well. Updating textdomain 'foo' with textdomain 'foo' didn't cause any problems.

Developing it further I'd like to see:

  • updateDomains: ['foo', 'bar'] only replaces domains in the whitelist (this is what develop branch does now).
  • updateDomains: true would replace ALL domains with the right textdomain. This helps when you don't know what all the existing domains are (perhaps PHP functions copied and pasted from multiple sources), but you know they should all be a single one for the project.
  • More feedback given (e.g. grunt.log.writeln) about what which domains were replaced, which were added, and in which files / lines (maybe only with `--verbose). I'll open a new Issue for this.

@bradyvercher
Copy link
Member

Thanks for the feedback, Gary, as well as the detailed instructions for testing. I should really get that incorporated in some documentation somewhere.

The updateDomains: true option sounds worthwhile, but also sounds like a decent amount of work. I'll open a separate issue for that.

@bradyvercher
Copy link
Member

These changes were released in v0.4.7

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

4 participants