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

Use core language pack translations where possible #534

Closed
fjarrett opened this issue May 16, 2014 · 10 comments

Comments

Projects
None yet
4 participants
@fjarrett
Copy link
Contributor

commented May 16, 2014

There are quite a few terms and phrases used in Stream that come from core. These translatable strings are already part of core language packs so it doesn't make sense for Stream to also have to provide translations for them.

Instead, we should use the default textdomain to indicate that the string is localized by core and that translations should not come from Stream.

EXAMPLES

esc_html__( 'Revision', 'default' );
esc_html__( 'Posts', 'default' );
esc_html__( 'Pages', 'default' );
esc_html__( 'Plugins', 'default' );
esc_html__( 'Menus', 'default' );

I know that we could just leave the textdomain param blank, but this could also be confused with a mistake as often times textdomains are forgotten, so it will be best to explicitly indicate that default should be used as an indicator that it has been checked and the translation is known to already exist in core.

@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented May 16, 2014

👍

@deckerweb

This comment has been minimized.

Copy link

commented May 31, 2014

I am totally against this practice and propose to change it back to the plugin's own textdomain!
Reasons:

  • the day WP core changes those strings, translations won't display
  • it makes custom translations for lots of users harder and more complex
  • it's against best practice, as textdomain should be UNIFIED and always be plugin's slug
  • those few strings save nothing on performance etc.

Example: if your WordPress install uses an informal German translation (which comes as defalt from .org) but the plugin would use an informal translation pack (as a custom user translation), user has to make sure there are no translation conflicts and in style/wording and this user has to override those strings from the plugin that use the - wrong - 'default' textdomain.
In some cases it could lead to not-displaying translations for those strings with the - wrong - 'default' textdomain. For example changing the string "Revision" in a custom user translation would also change that in the whole install - which may not always be wanted...

From my experience over the years such things make no sense, still some developers try it again and again. It only makes things more complex, even "worse" and has no benefit at all.

Every plugin and/ or theme/ child theme should use a unified textdomain for 100% of all strings and this should be the plugin's/ theme's slug in the very best case.

Thanks, Dave from Germany :)

@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2014

Hi Dave from Germany,

Thanks so much for your feedback. While I would normally agree with you on your points about different translation styles, part of the purpose of these strings within Stream is to provide a direct reference to another part of WordPress.

That is to say, unlike most other plugins, Stream definitely does want to match the active WordPress translation, so the user knows that when Stream uses the word Post, it means the exact same thing as WordPress core does. We would never want it to be different, regardless of translation style.

Does that make sense, or am I missing your point?

@deckerweb

This comment has been minimized.

Copy link

commented Jun 2, 2014

Hi Luke!
Ok, I didn't saw it from that perspective yet :) I fully understand your point and in such a special case it makes sense. My points were more of a general nature, though.

If it's only those 5 strings as stated in first post above you could make a little statement in your documentation/description why these are used directly from core with the 'default' textdomain. Then it's transparent and every (third-party) dev would know how to handle that.

Thanks for explanation!

Greetinx, Dave :)

@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2014

@ThemeBoy Would love your thoughts on this old issue.

@ThemeBoy

This comment has been minimized.

Copy link

commented Oct 1, 2014

Interesting discussion. Looks like there are a few different solutions each with their merits.

  1. Use different text domains i.e. keeps things simple and is the intended use of gettext functions.
  2. Use of contextual functions i.e. would be the proper way to merge all strings into a single text domain whilst retaining context.
  3. Defining a series of proprietary functions i.e. would prevent those strings from being picked up via gettext source keywords, and would minimize the number of strings contained in the language files.

The first 2 are perfectly valid solutions using gettext as intended. That being said, I agree with the point made above regarding the use of native text domains, and also believe that language files should be shipped with the code that references the same text domain. For those reasons I'm leaning toward the third solution.

@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2014

My concern with option 3 is that:
a) Adding additional code which is...
b) A bit of a hack (let's be honest), which means that we're...
c) Adding potential for contributors and Stream extension developers (and future me) to misuse a stream_e style function (which should only be used in very specific circumstances).

I'm leaning toward moving in the direction of Option 2. It's a bit more work on our end, but (secondo me) it seems like the right way.

@ThemeBoy

This comment has been minimized.

Copy link

commented Oct 1, 2014

You're right, it is a bit of a hack. Actually, option 2 could work out really well if you can filter out the instances of contextual strings referencing text domains when generating the PO template files. Best of both worlds :)

@deckerweb

This comment has been minimized.

Copy link

commented Oct 1, 2014

@ThemeBoy a few comments to your points:
for 1)
In my opinion it is NOT the intended usage of the Gettext functions, at least not in the WordPress environment. Their new "language pack" system (or dogma...) requires the use of a single textdomain throughout the plugin and/or theme, and that should always be the plugin's slug. Period. Otherwise, strings of different textdomains in that same plugin wouldn't be loaded and therefore not displayed as translations.
So this alternative shouldn't be used at all.

for 2)
Using context is the right way! It gives additional info for developers AND more importantly for translators as well, as these context comments will appear in tools like Poedit Editor, GlotPress and Transifex platform! This is really important! I translate for years, and again, this IS important for translators. Better use context more than less! :)

for 3)
This won't work at all!
It would only work if Stream plugin then would also LOAD the textdomain "woocommerce" (in that case) ADDITIONALLY to its textdomain. So you'll end up with 20 additional textdomains for all your integrations and need to load them... This is bad practice and bad user and translator experience as you have to offer then a .pot file for each textdomain.

Never go that route.

Only ever use one textdomain in your plugin, this is the slug of the plugin!
Unify all textdomains to this one.
Never "borrow" textdomains from Core or other plugins.
Implement compat to language pack system - it is by default, so don't "crash" it.

@lukecarbis

This comment has been minimized.

Copy link
Contributor

commented Oct 1, 2014

@deckerweb 👍

@fjarrett fjarrett removed their assignment Aug 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.