Re: Typo in quote.h

[Bruno Haible]

Hi Paul,

> >     * lib/quotearg.c (quotearg_slot_n_mem): Renamed from quotearg_n_options.
> 
> Why rename? The "slot_n_" part doesn't fit with the existing naming scheme, 
> which just uses "n_".

I try to fit it into the existing naming scheme, yes.

The existing functions which return the string in a slot (also the ones in
quote.h) have a suffix
  _n       with 'int n' argument
  _mem     when embedded NULs are supported
  _n_mem   with 'int n' argument, and embedded NULs are supported
So logically the function should have a suffix _n_mem.
But 'quotearg_n_mem' is already taken.
So the name should be something like   quotearg_<something>_n_mem
or quotearg_n_<something>_mem.

The functions which take a 'struct quoting_options const *' argument:
  quotearg_buffer
  quotearg_alloc
  quotearg_alloc_mem
and those that don't:
  quotearg
  quotearg_n
  quotearg_mem
  quotearg_n_mem
  quotearg_style
  quotearg_n_style
  quotearg_style_mem
  quotearg_n_style_mem
  quotearg_custom
  quotearg_custom_mem
  quotearg_n_custom
  quotearg_n_custom_mem

If you see "options" as a generalization of "style" and "custom", then
the systematic name should be 'quotearg_n_options_mem'. But then the
functions which return their result in a buffer or freshly allocated
should better be named

  quotearg_options_buffer      instead of  quotearg_buffer
  quotearg_alloc_options       instead of  quotearg_alloc
  quotearg_alloc_options_mem   instead of  quotearg_alloc_mem

> >     quotearg: Allow static init of 'struct quoting_options' variables.
> >     * lib/quotearg.h: Include <limits.h>.
> >     (struct quoting_options): Move to here from lib/quotearg.c. Use
> >     INT_WIDTH instead of INT_BITS.
> >     (QUOTING_OPTIONS_INIT): New macro.
> 
> I don't see the need for the macro 'QUOTING_OPTIONS_INIT (a, b)'. We're 
> assuming 
> C99 now, so users can simply write '{ .style = a, .flags = b }' which is 
> clearer 
> anyway and avoids macros.

What is the purpose of the functions set_quoting_style, set_char_quoting, etc.?
I think it is to hide the internals of the struct from the user. That is a good
thing, because it allows the internals to evolve in the future, without breaking
source-level backward compatibility. So I'm adding a comment

   The fields of this struct are considered private.  Instead of accessing
   them directly, use the accessors
     get_quoting_style, set_quoting_style,
     set_quoting_flags,
     set_char_quoting,
     set_custom_quoting
   declared below.

The macro QUOTING_OPTIONS_INIT is necessary to hide the internals of the struct.
If you encourage users to write

    struct quoting_options my_options = { .style = a, .flags = b };

then we cannot evolve the internals in the future.

Bruno