[Kimchi-devel] [PATCH V2] add a make check-local command to verify the i18n string format

[Zhou Zheng Sheng]

Good catch Christy!

I think it's possible to write an regular expression to match the format
specifiers precisely. We can just translate the general form of a
standard format specifier [1] into regular expression, and reverse the
last "type" part to catch all specifiers without a "type".

The original expression in the patch is
 %\([^\)]*\)[^0-9\.bcdeEfgGnosxX%]
Then we just change it to
 %\([^\)]*\)(.?[<>=^])?[+-]?#?0[0-9]*,?\(\.[0-9]+)?[^bcdeEfFgGnosxX%]
 (Caution: untested expression, it's just an example. Verify it yourself.)

Here the dot is matched in the "precision" part "(\.[0-9]+)?", so the
last part is just "type" containing no dot.

[1]
https://docs.python.org/2/library/string.html#format-specification-mini-language

on 2014/04/03 10:31, Sheldon wrote:
> On 04/02/2014 11:32 PM, Christy Perez wrote:
>> It looks like the expression needs a bit of tweaking still. I just
>> happened to try a string with a period next to the string format 's' and
>> that isn't caught.
>>
>> I changed "KCHREPOS0021E": _("Could not disable repository
>> %(repo_id)s."),
>> to "KCHREPOS0021E": _("Could not disable repository %(repo_id)."),
>> and didn't get an error with make check-local.
> For %(repo_id).5f is a right format string.
> 
> "KCHREPOS0021E": _("Could not disable repository %(repo_id).5f"),
> 
> let me improve the expression.
> 
>> Try: "KCHREPOS0021E": _("Could not disable repository %(repo_id)"),
>>
>> and you'll get an error about the string.
>>
>>
>> On Wed, 2014-04-02 at 14:37 +0800, shaohef at linux.vnet.ibm.com wrote:
>>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>>
>>> When I do i18n translation, I find some string format are wrong.
>>> So I add a check-local command to help the developer to check their
>>> string format.
>>>
>>> Every developers please run this command before submit your patch.
>>> Thanks.
>>>
>>> After you run this command, it may report some invalid string formats.
>>> $ make check-local
>>> Checking for invalid string formatting...
>>>      "KCHREPOS0018E": _("Could not write repository configuration file
>>> %(repo_file)"),
>>>
>>> You should check %(repo_file) is what you want.
>>>
>>> Ref:
>>> http://docs.python.org/2/library/string.html
>>>
>>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>>> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
>>> ---
>>>   Makefile.am | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index c68f050..915537d 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -73,7 +73,15 @@ PEP8_WHITELIST = \
>>>
>>>   SKIP_PYFLAKES_ERR = "\./src/kimchi/websocket\.py"
>>>
>>> +I18N_FILES = plugins/*/i18n.py \
>>> +    src/kimchi/i18n.py \
>>> +    $(NULL)
>>> +
>>>   check-local:
>>> +    @echo "Checking for invalid string formatting..."
>>> +    @grep -P "%\([^\)]*\)[^0-9\.bcdeEfgGnosxX%]" $(I18N_FILES) | \
>> Removing the '.' from the last match ([^0-9\bcdeEfgGnosxX%]) does catch
>> that case, but will that cause it to miss any others?
>>
>>> +        while read LINE; do echo "$$LINE"; false; done
>>> +    @echo "Checking for invalid string formatting successfully"
>> A tiny correction:
>> @echo "Checking for invalid string formatting successful."
>>
>>>       find . -path './.git' -prune -type f -o \
>>>           -name '*.py' -o -name '*.py.in'  | xargs $(PYFLAKES) | \
>>>           grep -w -v $(SKIP_PYFLAKES_ERR) | \
>>
>>
>>
> 
> 


-- 
Thanks and best regards!

Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou at linux.vnet.ibm.com
Telephone: 86-10-82454397