Re: quotearg: implement custom_quoting_style

[Bruno Haible]

Hi Joel,

> +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.

>  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.

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  <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.

Bruno