Hi Joel,
> +2009-08-22 Joel E. Denny <jde...@clemson.edu>
> +
> + 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 <jde...@clemson.edu>
>
> + 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.
> 2009-08-22 Joel E. Denny <jde...@clemson.edu>
>
> + 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.
Your API is the best that can be done without totally breaking the existing
quotearg API. (In an object-oriented world, we would combine the style and the
two quote strings into a single object. But that would change the function
signatures a lot.)
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 = "])". Can you call them
'left_quote' and 'right_quote', respectively? That would avoid this
misunderstanding.
- 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.
> + /* 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.
> 2009-08-22 Joel E. Denny <jde...@clemson.edu>
>
> + 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.
Bruno
