[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: quotearg: implement custom_quoting_style
From: |
Joel E. Denny |
Subject: |
Re: quotearg: implement custom_quoting_style |
Date: |
Sun, 23 Aug 2009 12:29:18 -0400 (EDT) |
User-agent: |
Alpine 1.00 (DEB 882 2007-12-20) |
On Sun, 23 Aug 2009, Bruno Haible wrote:
> Hi Joel,
Hi Bruno.
> > +2009-08-22 Joel E. Denny <address@hidden>
> > +
> > + quotearg-tests: test escaping of embedded locale quotes
> > + * tests/test-quotearg.c (struct result_strings): Add member for
> > + new input.
> > + (LQ_ENC, RQ_ENC, RQ_ESC): New macros.
> > + (inputs): Add new input.
> > + (results_g): Add expected results.
> > + (flag_results): Likewise.
> > + (locale_results): Likewise.
> > + (compare_strings): Check those.
>
> Thanks. It's good to see the testsuite coverage increase.
>
> > 2009-08-22 Joel E. Denny <address@hidden>
> >
> > + quotearg: fix right quote escaping when it's in quote_these_too
> > + * lib/quotearg.c (quotearg_buffer_restyled): Upon seeing a right
> > + quote, be sure to prepend only one backslash.
> > + * tests/test-quotearg.c (use_quote_double_quotes): New function.
> > + (main): Test it.
>
> Thanks as well. We were not aware of this bug.
Thanks. Should I go ahead and push those?
> > 2009-08-22 Joel E. Denny <address@hidden>
> >
> > + quotearg: implement custom_quoting_style
> > + * lib/quotearg.c: (struct quoting_options): Add left and right
> > + fields to store quotes.
> > + (set_custom_quoting): New public function.
> > + (quotearg_buffer_restyled): Add left and right arguments,
> > + handle them very much like locale quoting, and update all uses.
> > + (quotearg_n_custom): New public function.
> > + (quotearg_n_custom_mem): New public function.
> > + (quotearg_custom): New public function.
> > + (quotearg_custom_mem): New public function.
> > + * lib/quotearg.h: Prototype and document new public functions.
> > + (enum quoting_style): Add custom_quoting_style member and
> > + document.
> > + * tests/test-quotearg.c (custom_quotes): New array.
> > + (custom_results): New array.
> > + (main): Extend to test custom quoting.
>
> Looks good as well. I agree that the previously available choices for
> the quote strings - either hardcoded, or from the translated message catalog -
> are not sufficient for all use-cases.
Thanks for the prompt review.
> Just two points:
>
> - Can you please avoid 1-letter identifiers as function parameters?
> 'l' and 'r', in particular. At the first glance, I thought it would
> be possible to set l = "[(" and r = "])".
Why isn't that possible?
quotearg_custom ("[(", "])", "[(joel's])\tstring");
produces
"[([(joel's\\])\\tstring])"
Is that not what you expect, or did I miss your point?
> Can you call them
> 'left_quote' and 'right_quote', respectively? That would avoid this
> misunderstanding.
Regardless of the above rationale, I agree with the better names.
> - The 'if (quoting_style != custom_quoting_style) {' code should be
> moved to before the big 'TRANSLATORS:' comment. You know that
> this comment is meant to be extracted by xgettext, and xgettext
> extracts it only if there is no other non-empty line between the
> comment and the gettext() invocation.
I really need to study internationalization more. I'll fix it. Thanks.
> > + /* Like c_quoting_style except use the custom quotation marks set by
> > + set_custom_quoting.
>
> This should be "Like clocale_quoting_style", not "Like c_quoting_style":
> Because c_quoting_style also handles trigraphs, which custom_quoting_style
> doesn't.
I missed that. I copied those comments from the comments on
clocale_quoting_style, which don't mention the trigraph distinction.
I'll fix that too.
> > 2009-08-22 Joel E. Denny <address@hidden>
> >
> > + quotearg: document limitations of quote_these_too
> > + * lib/quotearg.c (quotearg_buffer_restyled): Add comments where
> > + those limitations are created.
> > + * lib/quotearg.h (set_char_quoting): Document that digits and
> > + letters that are special after backslash are not permitted.
> > + (quotearg_char): Cross-reference set_char_quoting documentation.
> > +
>
> OK, good.
I'll wait to push this until after the custom_quoting_style patch.
- quotearg: implement custom_quoting_style, Joel E. Denny, 2009/08/22
- Re: quotearg: implement custom_quoting_style, Bruno Haible, 2009/08/23
- Re: quotearg: implement custom_quoting_style,
Joel E. Denny <=
- Re: quotearg: implement custom_quoting_style, Joel E. Denny, 2009/08/23
- Re: quotearg: implement custom_quoting_style, Bruno Haible, 2009/08/23
- Re: quotearg: implement custom_quoting_style, Joel E. Denny, 2009/08/23
- Re: quotearg: implement custom_quoting_style, Bruno Haible, 2009/08/23
- Re: quotearg: implement custom_quoting_style, Joel E. Denny, 2009/08/23