This adds a textgroup parameter everywhere so other modules can add strings to the list and have them in the right text group (other than 'default').
The patch is simple though it needs to add the field everywhere:
- Textgroup parameter for l10_client_add_string_to_page()
- This makes the strings array indexed by textgroup and source
- Adds this new field in _l10n_client_dom_strings()
- Two lines of javascript to have the value submitted.
My use case is i18n module (which adds strings to l10n client's list) but for different text groups. With this one, l10n client can work with i18n (i18nstrings) to translate user defined strings. This is related to: #287210: Some new features ported from 5.x version
Comment | File | Size | Author |
---|---|---|---|
#25 | add-textgroup-d7.patch | 7.44 KB | Gábor Hojtsy |
#20 | l10n_client-port-361147.patch | 7.59 KB | dawehner |
#14 | l10n_client_i18n_textgroup_2.patch | 7.15 KB | dawehner |
#11 | l10n_client_i18n_textgroup_2.patch | 7.18 KB | miro_dietiker |
#5 | l10n_client.i18n_textgroup.patch | 6.47 KB | jhedstrom |
Comments
Comment #1
Jose Reyero CreditAttribution: Jose Reyero commentedComment #2
mathieu CreditAttribution: mathieu commentedPatch still worked, but didn't apply cleanly anymore.. so here's a new version for l10_client-6.x-1.7.
Comment #3
j0nathan CreditAttribution: j0nathan commentedsubscribing...
Comment #4
neurovation.kiwi CreditAttribution: neurovation.kiwi commentedHi folks,
just added another change - cause there was some problem with passing on the real textgroup instead of the default 'default'.
enjoy!
Comment #5
jhedstromThe patch in #4 doesn't apply cleanly (and doesn't apply completely on my machine). Attached is the same patch, rerolled against the latest version.
I think this patch still needs work because it has some pretty serious usability issues for translators. The main issue being that non-default sources are listed twice, without any visual indicator as to which text group they are in. If the translator only translates one of them, the translations for the given string are in a hit-and-miss sort of state.
Comment #6
neomenlo CreditAttribution: neomenlo commentedIs there a reason why the groups have to be separated?
I'd like to see them all in the same list, to keep things very simple.
Comment #7
aufumy CreditAttribution: aufumy commentedin case of collision of words, such as 'associate' noun or verb.
Also view http://drupal.org/node/307338
Comment #8
miro_dietikerTested this in production and checked code. Looks pretty neat.
Comment #9
miro_dietikerSorry, this RTBC was wrong.
I've tested a lot and all patches don't apply cleanly anymore.
So i've built a new version which is tested and working in production anymore.
This time against -dev with updated code.
I can't see any double text string output. Everything is clean in my UI - as long as you apply the i18n text group patch:
#304434: i18nstrings l10n_client support.
Once fixed this will bring us much further in translation workflows. Please review!
Comment #10
robby.smith CreditAttribution: robby.smith commented@miro_dietiker - sorry, you mention you've built a new version..did you forget to upload the patch? or has this issue been moved to #304434: i18nstrings l10n_client support
thanks
Comment #11
miro_dietikersorry.. thought i've uploaded the patch.
here it is. hopefully still the right version.
Comment #12
maijs CreditAttribution: maijs commentedNope, still can't apply #11 to 6.x-dev version.
Comment #13
dawehnerThis is a new version, which applies cleanly.
Before i reviewed the code, and it looked very fine. So this could be made rtbc by someone else.
Comment #14
dawehnerThis is a new version, which applies cleanly.
Before i reviewed the code, and it looked very fine. So this could be made rtbc by someone else.
Comment #15
miro_dietikerWell, thx, looks fine to me. Minor changes happened and many people already tested the base patches.
Comment #16
Gábor HojtsyGreat, committed this in. I've fixed one issue I found though. In this code segment:
I think the first 'default' was an issue since we'd have checked an array which we are not working on then :) So modified to check the same array:
Comment #17
miro_dietikerGlad to see such progress here. Sure you're right gabor. I'm sure this will fix some of our issues we still had!
Right updated to this version which seems to work smooth.
(Only missing per-user-disable feature of l10n_client which is in issue queue now...)
Thank you!
Comment #18
Gábor HojtsyYes, I'll look at per-user disable next :)
Comment #19
Gábor HojtsyNeeds to be ported / committed to 7.x as well.
Comment #20
dawehnerPS: It helps if you post the exact patch you commited.
Currently the d7 version seams to be broken(the js interface);
This patch does only gives out the stuff as needed
Comment #21
dawehnerupdate status
Comment #22
Gábor HojtsyAs you've already noted, this includes #773812: Fix warnings which was just committed. So we can remove that. Its not part of the port. Also, comparing the D6 and D7 changeset, looks like you removed this line but provided no replacement?
Thanks for working on this!
Comment #23
kiprasm CreditAttribution: kiprasm commentedLol, i was working on the same thing (support for textgroups other then the hardcoded 'default') about a week ago (and for the same reason as well - im using i18nstrings), used it throughout the week, and was just about to post the changed files here, but i see this is already done :)
Hopefully this will be merged into the l10n_client 6.x stable version soon, so i will be able to compare it to my version, just out of pure interest :)
UPDATE:
Another lol - now i see it has already been commited just about a week ago :D
Comment #24
Gábor Hojtsy@kiprasm: yes, it was committed to the Drupal 6 version and released over a week ago in 6.x-1.8: http://drupal.org/node/872024
Comment #25
Gábor HojtsyWorked on this one and ported the functionality entirely to Drupal 7. Drupal 7 does not have automated IDs for hidden fields anymore, so I needed to add a class. Going to backport that to Drupal 6, since that might cause bugs like #940142: CCK translation creates strings in the "Built in interface" text group, incompatible with translation_table and other methods.
Comment #26
Gábor HojtsyBackported the textgroup class in #940142: CCK translation creates strings in the "Built in interface" text group, incompatible with translation_table and other methods.