You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mattias Engdegård <ma...@bredband.net> on 2013/03/29 00:15:45 UTC
[PATCH] Make conflict message translatable
The conflict message "local %s %s, incoming %s %s upon %s" in
svn_cl__get_human_readable_tree_conflict_description is not correctly
prepared for localisation.
First, two of the parameters are inserted untranslated (in English),
making proper translation impossible; second, piecing together a
string from individual words does not work in general for many
translations, because of varying word order and concord.
The standard solution is to provide whole strings for each case, but
the number of combinations is probably just too big for that:
2*9*2*4*3 = 432, if all are possible (probably untrue). As a
compromise to keep it manageable, this patch only translates each sub-
phrase separately, resulting in 2*9 + 2*4 + 3 = 29 short strings.
The patch assumes that svn_node_none, svn_node_unknown or
svn_wc_operation_none cannot occur here. Please confirm or refute.
The produced (English) strings are not changed, although doing so
should perhaps be considered since these particular messages seem to
be particularly difficult to understand.
Implementation-wise, I preferred using switch statements to mucking
about with tables, since it made the code simpler and statically safer
(thanks to gcc's exhaustiveness check when switching on enums).
[[[
Make the conflict message "local %s %s, incoming %s %s upon %s"
translatable by constructing it from longer parts, each translated,
instead of single words (some of which were not translated at all).
* subversion/svn/cl-conflicts.c
(svn_cl__get_human_readable_tree_conflict_description): Build
string out
of translated sub-phrases rather than from single words.
(local_reason_str, incoming_action_str, operation_str): New.
(map_conflict_action_human, map_conflict_reason_human,
action_str, reason_str): Removed.
]]]
Re: [PATCH] Make conflict message translatable
Posted by Julian Foad <ju...@btopenworld.com>.
Mattias Engdegård wrote:
> 2 apr 2013 kl. 15.51 skrev Julian Foad:
>> FAIL: upgrade_tests.py 24: wc with add-add and del-del tree conflicts
>>
>> That's failing in local_reason_str() with kind=none and reason=deleted,
>> so at least we need that function to handle that case.
>
> Quel blamage! I'm really sorry - I thought I had run all the tests, but
> apparently not.
> There is no excuse for that.
No problem, I do that sometimes.
>> This indicates that perhaps the only way to get kind=none is an upgrade
>> from a previous WC format. We could handle such unusual cases by constructing a
>> response from the separate parts, and not attempt to use different messages for
>> every possible combination. The important thing is to use well written messages
>> for the common cases. The unusual or impossible cases can just have some
>> inelegant construction like we were doing before, and then we'd be sure to
>> handle any case that might arise rather than throwing an error.
>
> I agree. Here is a new patch which does exactly that. It re-uses the XML strings
> for the unusual case since there really is no point in translating them.
>
> [[[
> Make the conflict message "local %s %s, incoming %s %s upon %s"
> translatable by constructing it from longer parts, each translated,
> instead of single words (some of which were not translated at all).
>
> * subversion/svn/cl-conflicts.c
> (svn_cl__get_human_readable_tree_conflict_description): Build string out
> of translated sub-phrases rather than from single words.
> (local_reason_str, incoming_action_str, operation_str): New.
> (map_conflict_action_human, map_conflict_reason_human,
> action_str, reason_str): Removed.
> ]]]
Committed in r1465299. Thanks!
- Julian
Re: [PATCH] Make conflict message translatable
Posted by Mattias Engdegård <ma...@bredband.net>.
2 apr 2013 kl. 15.51 skrev Julian Foad:
> A quick update. I tried your patch and the test suite fails here:
>
> FAIL: upgrade_tests.py 24: wc with add-add and del-del tree conflicts
>
> That's failing in local_reason_str() with kind=none and
> reason=deleted, so at least we need that function to handle that case.
Quel blamage! I'm really sorry - I thought I had run all the tests,
but apparently not.
There is no excuse for that.
> This indicates that perhaps the only way to get kind=none is an
> upgrade from a previous WC format. We could handle such unusual
> cases by constructing a response from the separate parts, and not
> attempt to use different messages for every possible combination.
> The important thing is to use well written messages for the common
> cases. The unusual or impossible cases can just have some inelegant
> construction like we were doing before, and then we'd be sure to
> handle any case that might arise rather than throwing an error.
I agree. Here is a new patch which does exactly that. It re-uses the
XML strings for the unusual case since there really is no point in
translating them.
[[[
Make the conflict message "local %s %s, incoming %s %s upon %s"
translatable by constructing it from longer parts, each translated,
instead of single words (some of which were not translated at all).
* subversion/svn/cl-conflicts.c
(svn_cl__get_human_readable_tree_conflict_description): Build
string out
of translated sub-phrases rather than from single words.
(local_reason_str, incoming_action_str, operation_str): New.
(map_conflict_action_human, map_conflict_reason_human,
action_str, reason_str): Removed.
]]]
Re: [PATCH] Make conflict message translatable
Posted by Julian Foad <ju...@btopenworld.com>.
Mattias Engdegård wrote:
> 29 mar 2013 kl. 03.03 skrev Julian Foad:
>> Mattias Engdegård wrote:
>>> The patch assumes that svn_node_none, svn_node_unknown or
>>> svn_wc_operation_none cannot occur here. Please confirm or refute.
>>
>> I haven't checked whether these can occur in this message. svn_node_unknown
>> can certainly occur for the local kind of node -- such as in a tree conflict
>> when a merge tries to edit a path which doesn't exist in the local working
>> copy. I *think* the node kind that appears in these messages can be 'none'
>> for this reason I can check this and fix if necessary and commit it,
>> probably on Monday if nobody does it sooner.
>
> Please do - I haven't been able to produce such a situation myself, but
> I'm unsure of the invariants in effect at this point.
A quick update. I tried your patch and the test suite fails here:
FAIL: upgrade_tests.py 24: wc with add-add and del-del tree conflicts
That's failing in local_reason_str() with kind=none and reason=deleted, so at least we need that function to handle that case.
This indicates that perhaps the only way to get kind=none is an upgrade from a previous WC format. We could handle such unusual cases by constructing a response from the separate parts, and not attempt to use different messages for every possible combination. The important thing is to use well written messages for the common cases. The unusual or impossible cases can just have some inelegant construction like we were doing before, and then we'd be sure to handle any case that might arise rather than throwing an error.
Does that sound good and, if so, would you like to change the patch to do that?
- Julian
>> My only other suggestion would be to include the comma in the English
>> strings instead of in the format string -- _("local file edit,")
>> instead of _("local file edit") -- so that the other translations can
>> use different (or no) punctuation.
>
> To me it seemed unlikely that the punctuation would vary between different
> cases, but I could be wrong about that. Changing it in the way you propose
> wouldn't do any harm, so please do that if you like.
>
> Thanks for looking at the patch!
Re: [PATCH] Make conflict message translatable
Posted by Mattias Engdegård <ma...@bredband.net>.
29 mar 2013 kl. 03.03 skrev Julian Foad:
> Mattias Engdegård wrote:
>
>> The patch assumes that svn_node_none, svn_node_unknown or
>> svn_wc_operation_none
>> cannot occur here. Please confirm or refute.
>
> I haven't checked whether these can occur in this message.
> svn_node_unknown can certainly occur for the local kind of node --
> such as in a tree conflict when a merge tries to edit a path which
> doesn't exist in the local working copy. I *think* the node kind
> that appears in these messages can be 'none' for this reason I can
> check this and fix if necessary and commit it, probably on Monday if
> nobody does it sooner.
Please do - I haven't been able to produce such a situation myself,
but I'm unsure of the invariants in effect at this point.
> My only other suggestion would be to include the comma in the
> English strings instead of in the format string -- _("local file
> edit,") instead of _("local file edit") -- so that the other
> translations can use different (or no) punctuation.
To me it seemed unlikely that the punctuation would vary between
different cases, but I could be wrong about that. Changing it in the
way you propose wouldn't do any harm, so please do that if you like.
Thanks for looking at the patch!
Re: [PATCH] Make conflict message translatable
Posted by Julian Foad <ju...@btopenworld.com>.
Mattias Engdegård wrote:
>T he conflict message "local %s %s, incoming %s %s upon %s" in
> svn_cl__get_human_readable_tree_conflict_description is not correctly prepared
> for localisation.
[...]
Thanks, Mattias. Your patch certainly looks to me like a good improvement.
> The patch assumes that svn_node_none, svn_node_unknown or svn_wc_operation_none
> cannot occur here. Please confirm or refute.
I haven't checked whether these can occur in this message. svn_node_unknown can certainly occur for the local kind of node -- such as in a tree conflict when a merge tries to edit a path which doesn't exist in the local working copy. I *think* the node kind that appears in these messages can be 'none' for this reason I can check this and fix if necessary and commit it, probably on Monday if nobody does it sooner.
> The produced (English) strings are not changed, although doing so should perhaps
> be considered since these particular messages seem to be particularly difficult
> to understand.
>
> Implementation-wise, I preferred using switch statements to mucking about with
> tables, since it made the code simpler and statically safer (thanks to gcc's
> exhaustiveness check when switching on enums).
Looks good at a quick read-through.
My only other suggestion would be to include the comma in the English strings instead of in the format string -- _("local file edit,") instead of _("local file edit") -- so that the other translations can use different (or no) punctuation.
- Julian
>
> [[[
> Make the conflict message "local %s %s, incoming %s %s upon %s"
> translatable by constructing it from longer parts, each translated,
> instead of single words (some of which were not translated at all).
>
> * subversion/svn/cl-conflicts.c
> (svn_cl__get_human_readable_tree_conflict_description): Build string out
> of translated sub-phrases rather than from single words.
> (local_reason_str, incoming_action_str, operation_str): New.
> (map_conflict_action_human, map_conflict_reason_human,
> action_str, reason_str): Removed.
> ]]]
>