On Sun, 23 Aug 2009, Bruno Haible wrote: > Hi Joel,
Hi Bruno. > > +2009-08-22 Joel E. Denny <[email protected]> > > + > > + 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 <[email protected]> > > > > + 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 <[email protected]> > > > > + 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 <[email protected]> > > > > + 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.
