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

[RFC] Translation stuff #5056

Closed
rarila opened this issue Mar 20, 2016 · 16 comments
Closed

[RFC] Translation stuff #5056

rarila opened this issue Mar 20, 2016 · 16 comments
Labels
RFC Request For Comments translation

Comments

@rarila
Copy link
Contributor

rarila commented Mar 20, 2016

Ok, here some stuff we really need to to – and I really think we should do it for 3.0 – to bring that translation stuff ahead and work on the switches. Themes/Extensions translations to be handled later.

We have two things to tackle:

  • Translations get overwritten on update.
  • Translations are (somehow) handled on Crowdin now.

This changes how the internal editor should work.

So my suggestions:

  • We create /config/translations/…
  • Our internal translation tool only works on that directory
  • Handled there are theme translations (to be moved later), contenttype messages and other stuff from contenttypes.yml
  • app/resources/translations/… just has the fixed translations for the backends and serves as source to copy theme and contenttype translations over to config/…
  • On the dev side en_GB is handled as the master translation and is the only one that get’s updated when there are new strings or strings to be removed or other changes. All other translations are handled on Crowdin.
  • To do so this scanning and updating isn’t done in the backend anymore, we do it with a nut command (I think grunt isn’t suited here)

Result:

  • The translation tool in the backend now only scans theme files and contenttypes.yml and writes the result to /config/translations/…
  • After a dev added/changed some translation stuff he runs a nut command and that then updates the files in app/resources/translations/en_GB/…. We distinct there between stuff to directly be used (backend) and (for now) theme and the contenttype translations for the contenttypes.yml.dist. The latter are just be used for copying/fallback.

I hope this doesn’t sounds too confusing. If I get an ok I might start with splitting messages.en.GB.yml into it's different parts, that are in there now. I guess things get a bit clearer then.

@GwendolenLynch GwendolenLynch added the RFC Request For Comments label Mar 20, 2016
@bobdenotter
Copy link
Member

I agree with all the things you say in here, except for one part:

we should do it for 3.0

We are this close to beta, and we really should not be starting a refactoring this close to a planned stable release.

@GwendolenLynch
Copy link
Contributor

Translations are (somehow) handled on Crowdin now.

This is very much not the case

@GwendolenLynch
Copy link
Contributor

ping @CarsonF

OK… so I have had a few discussions with Carson over the last few days about this, with respect to how GMO are handling it internally. They have a very large base of applications that require very broad (and managed) translation handling. Given that GMO has an employee on the core team and extensive experience in managing it, I am very keen to get their guidance.

We are this close to beta, and we really should not be starting a refactoring this close to a planned stable release.

I agree 100% with you @bobdenotter, but there may be some low/no risk steps we can take during the beta phase… and given the activity around this of late, it doesn't hurt to at least start the dialogue 😄

@rarila
Copy link
Contributor Author

rarila commented Mar 20, 2016

As afaik the translations get overwritten on update, I call that a Blocking release. How will one explain that to the user how to save and restore them on every update or why the hours of work are suddenly got lost?

@GwendolenLynch
Copy link
Contributor

@rarila… mate… 😱 🎈

This has been the case since… always. We know we want a better solution, we know we need to create a better solution… and you and I agreed to work on a branch together for Bolt 2.1… well over a year ago and I am still waiting on you 😉

(I know you've been kicking butt with the JavaScript refactor over the last year or so… but if we continue the "refactor everything" approach, we'll become Drupal)

@GwendolenLynch
Copy link
Contributor

By the way… One of the things I was discussing with Carson, and why I want to give him a second to chime in, is that your proposal comes close to what we're discussing… Transiflex in particular requires a "root" string set.

@rarila
Copy link
Contributor Author

rarila commented Mar 20, 2016

By doing these things not much can be broken as we just change wich paths to scan and which paths to load with twig loader

@CarsonF
Copy link
Member

CarsonF commented Mar 20, 2016

Really @rarila's point about translations being overridden points to a bigger issue. Our app folder contains files for two different purposes. cache, config, and database are folders for specific user installations. Where resources, app/src, theme_defaults, and view are for bolt core and are not meant to be changed by user. The second set of folders should be moved outside of the app folder and into a bolt or core or [insert name here] folder. While they are not meant to be changed by the user, they are meant to be overridden (see open/close from S.O.L.I.D. principles).

This is a major change so I have been waiting to bring it up until we do a major release (and then there was the whole clusterfcuk with 2.3/3.0). For this reason, I vote we wait until we are ready to do this entire project structure change all at once. I'm going to RFC this separately.

Additionally, most of our translation code needs to be rewritten. This is a huge undertaking and now is definitely NOT the time to be doing that as we are preparing for beta.

There are also several things not agreed upon regarding translations which should be worked out first. The debate between Crowdin vs Transifex, which BTW has not been decided. The debate between 2 letter and 4 letter identifiers.


Transifex (and it looks like Crowdin as well) uses Ruby On Rails yaml format which requires the root node of the YAML file to be the locale.

en:
    foo: bar

This requires a custom file loader for symfony's translator to ignore this. It's is pretty straight forward though, and I already have one written for GMO.

Transifex's YAML parser is a bit stricter than Symfony's which caused a few problems at first.

I don't want to go into too much detail but basically you create a .tx/config file where you define your file mappings. Then you just use their client: tx push and tx pull. It's pretty straight forward.


Let me state again, that I have yet to be convinced that Crowdin is any different or better than Transifex. And I don't want anyone to assume that this decision as been made. I'm not saying that I wouldn't pick Crowdin, but I haven't had a chance to look at it.

I do also want to emphasize that we should definitely not be trying to integrate user translation files with these external services. They are not core files and they would need a different project in Transifex. This should be implemented as an extension, as there a use-case for this, but it is not for everyone.

@cyanonoob
Copy link

Just dropping in with some thoughts:
As it's important for some languages it's important to be able to have different wordings for the same string, depending on for instance the name of a content-type, perhaps the following might be possible:
Adding another (optional) field to content-types, for instance for the label text for the Add new [content-type name]-button, and then (possibly) adding translations for this per content type in the proposed (?) app/resources/translations folder as well?
For languages where this isn't necessary, the default fallback with no specified labels would do, for other languages you could either specify them in your content-type definition (though perhaps not as clean, it would be easier for small local-scale projects), or provide translation(s) in app/resources (which would, per the original proposal, not get overwritten on update).
Disclaimer: I'm very new to Bolt, and though I haven't had a great time there, am very influenced by how dirty word WordPress handles things. Though I am in no way hoping to repeat or recreate most of that, of course.

@GwendolenLynch
Copy link
Contributor

@cyanonoob This RFC is somewhat out of date with where things are actually at.

In v3 we dropped support for git installation, and moved the end-user install method to a Compeser based one, so what is referred to here as app/resources/translations is now vendor/bolt/bolt/app/resources/translations and from an end user point of view is immutable, and is always overwritten on update.

With Bolt 3.3 we removed ContentType translations, and in its place we made site specific translations work properly from the site's own app/translation directory … details in #6802

@SvanteRichter
Copy link
Contributor

@cyanonoob @GawainLynch Sorry if this was the wrong place to drop this in, @cyanonoob asked about it in slack and I said this was a good place to comment since it is open and is an RFC. If this is that out of date perhaps we should open a new RFC or append this one with the details that have changed?

@GwendolenLynch
Copy link
Contributor

GwendolenLynch commented Sep 16, 2017

Oh yeah, I read emails & issued before I went over the previous night's Slack conversations.

By out of date, I just meant some of the information in here. I think it is valid enough to keep this one running with the history in it.

@stale
Copy link

stale bot commented Nov 20, 2017

This issue has been automatically marked as stale because it has not had recent activity. Maybe this issue has been fixed in a recent release, or perhaps it is not affecting a lot of people?
It will be closed if no further activity occurs, because we like to keep the issue queue concise and actual.
If you think this issue is still relevant, please let us know. Especially if you’d like to help fix the issue, either by helping us pinpointing the cause of a bug, or in implementing a fix or new feature.

@stale stale bot added the stale Stale issues & PRs flagged for closing label Nov 20, 2017
@SvanteRichter
Copy link
Contributor

This one needs an answer/discussion IMO so keeping it open. Feel free to close if that seems better,

@stale stale bot removed the stale Stale issues & PRs flagged for closing label Nov 20, 2017
@GwendolenLynch
Copy link
Contributor

The old translation layer has been completely removed for v4, and replaced with Symfony's … it is a whole different ballgame now 😉

@SvanteRichter
Copy link
Contributor

Okay, closing then. I'm sure the discussion is better had when those changes are in or up on review.

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

No branches or pull requests

6 participants