9:54 p.m.
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-l...
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(a)linux.vnet.ibm.com wrote:
>> From: ShaoHe Feng <shaohef(a)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(a)linux.vnet.ibm.com>
>> Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
>> Signed-off-by: Christy Perez <christy(a)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(a)linux.vnet.ibm.com
Telephone: 86-10-82454397