Closed (fixed)
Project:
Translation template extractor
Version:
6.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
17 Sep 2008 at 19:34 UTC
Updated:
7 Sep 2009 at 13:10 UTC
Jump to comment: Most recent file
Comments
Comment #1
douggreen commentedThis patch does an even better job at converting the potx warnings into coder messages.
Comment #2
gábor hojtsyGreat, thanks for putting in effort to propagating the changes :) I am glad to implement the coder review hook and help people fix translatability issues. I have a few comments on the patch:
- There is no guarantee that the translation of the error message will keep the $file:$line notation, so we need to move to %location and then replace %location with $file:$line to ensure this format is kept.
- Same applies for your matching of ' in ...' which is based on the English original string. I'd suggest you match for : and whitespace followed by non-whitespace stuff before it and numbers after it. That should identify the file name/line properly regardless of translation.
- Although possibly rare, the theming of %filename could also use something else but
<em>, so it might make sense to look at the theme('placeholder') processed version.- There are other instances of potx_error() which would need fixing with this in mind.
Thanks again for helping fix the code review code! I hope my suggested improvements make this change more translation and theme agnostic, and that you can keep up the work to improve the patch :)
Comment #3
gábor hojtsyAlso on better coder integration via providing better error messages to aid developers here: http://drupal.org/node/312523 (related, not duplicate)
Comment #4
stella commentedI don't have time to help improve on the patch, but I can certainly review / test future instalments.
Comment #5
gábor hojtsyOk, let's check this one out. It introduces a new POTX_STATUS_STRUCTURED status handling mode, in which case, it collects the error messages, line numbers and file names in a structured array, not in blobs of text. This also led to generalization in the error messages. Now all of them have the file name and line at the end in non-structured reporting modes, appended automatically.
I've also fixed phpdoc on potx_status() and the status constants, and used the wording "Invalid localization code" instead of "Invalid marker content". We have http://drupal.org/node/312523 for better error messages, so I am not going to go into that right now, this was a logical and slight improvement.
The coder integration gets much simpler due to these changes, so it does not need to reverse engineer the file and line number from the text, it does not need to remove these, and it does not need to replace stuff like newlines and asterisk marks, which were only there for the command line reporter originally and kept being there ever since. Way better IMHO. This clears out all my concerns from #2, so as long as coder folks can verify this works fine, I am happy to commit this.
Comment #6
stella commentedExcellent! That works well. :)
However, the patch from #312523: Better error messages for potx errors / code reviews. now no longer applies. So will re-roll that one.
Comment #7
gábor hojtsyGreat, committed to the 6.x-2.x branch, so we can keep going with the better messages there. Moving on to http://drupal.org/node/312523
Since the new potx_status() API was introduced in 6.x-2.x, it might be pointless to backport these changes to 5.x, since it would result in highly separate code paths. Marking to be ported to 5.x-2.x for now, and we will see later.
Comment #8
gábor hojtsyDoh. A considerable glitch in this patch was that coder numbers lines from 0 and potx does from 1 (as in most editors translators would use), so we need to decrement the line number we provide coder with or we get the next line as part of the error message (as my test showed). Applied this follow up patch and it runs much better now.
Comment #9
gábor hojtsyAlso related comments and screenshots on http://drupal.org/node/312523#comment-1057817
Comment #10
gábor hojtsyDrupal 6 already has the above patch committed, and Drupal 7 has different code structure for finding out the line number. Also, the Drupal 5 version is not maintained anymore, so marking back to fixed.