Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib

[Akim Demaille]

Le 27 janv. 2012 à 19:25, Paul Eggert a écrit :

> On 01/27/2012 02:38 AM, Akim Demaille wrote:
>> it seems to me that the
>> simplest path is to never rely on quotearg for the quotes
> 
> Yes, that sounds right to me too.  To be honest I've never been
> a big fan of quotearg (even though I wrote it!).

To be honest, I like it a lot, and did not write it :)

I went ahead and did that, but looking at the result, it seems
all wrong to me.

I really like the idea that quote() be in charge of the quotes:

- this way we are sure that the occurrences of the right quote
  are properly escaped without escaping uselessly many characters,

- there is a single place to decide whether to issue quotes or
  not (which was a wish of mine that I did not know that
  quotearg supported: I just discovered QA_ELIDE_OUTER_QUOTES
  which I like a lot),

- this provided nice quotes to people using straight POSIX/C
  locale.

I don't think it goes against the GCS, at least in the spirit:
the point is to support localized quotes, which is what
locale_quoting_style offers.  And it is certainly better for
translators not to risk to have multiple styles of quotes.

In addition, I found that when trying to be more consistent,
I was breaking efforts made by Joel in using quotes where
appropriate, for instance:

>   \\(.|\n)      {
>     char const *p = yytext + 1;
>     /* Quote only if escaping won't make the character visible.  */
>     if (isspace ((unsigned char) *p) && isprint ((unsigned char) *p))
>       p = quote (p);
>     else
>       p = quotearg_style_mem (escape_quoting_style, p, 1);
>     complain_at (*loc, _("invalid character after \\-escape: %s"), p);
>   }

(which is something that I will simplify later thanks to
QA_ELIDE_OUTER_QUOTES by the way).

To address the test-suite issue, I propose the following:

> enum quoting_style bison_quoting_style = locale_quoting_style;
> 
> void
> quote_init (void)
> {
>   if (getenv ("BISON_C_QUOTING_STYLE"))
>     bison_quoting_style = shell_quoting_style;
>   set_quoting_style (0, QA_ELIDE_OUTER_QUOTES);
> }
> 
> /* Return an unambiguous printable representation of NAME,
>    allocated in slot N, suitable for diagnostics.  */
> char const *
> quote_n (int n, char const *name)
> {
>   return quotearg_n_style (n, bison_quoting_style, name);
> }

There remain a couple of issues on which I'd like to have
your opinion Paul:

- locale_quoting_style does not seem to obey to
  QA_ELIDE_OUTER_QUOTES.  Is this on purpose?

>  $ ./_build/debug/tests/bison '""'.y -o /tmp/f
> bison: cannot open file ‘"".y’: No such file or directory
>  $ ./_build/debug/tests/bison ff.y -o /tmp/f
> bison: cannot open file ‘ff.y’: No such file or directory
> 
>  $ BISON_C_QUOTING_STYLE=1 ./_build/debug/tests/bison ff.y -o /tmp/f
> bison: cannot open file ff.y: No such file or directory
>  $ BISON_C_QUOTING_STYLE=1 ./_build/debug/tests/bison '""'.y -o /tmp/f
> bison: cannot open file '"".y': No such file or directory


- Is there a consensus among gnulibers on single quotes vs.
  double quotes in error messages?

- So I am in favor of removing all the quotes from the format
  strings, for instance (demonstrated above):

>    if (!ptr)
> -    error (EXIT_FAILURE, get_errno (), _("cannot open file '%s'"), name);
> +    error (EXIT_FAILURE, get_errno (), _("cannot open file %s"), quote 
> (name));

  does everybody agree?

- I used shell_quoting_style for a fast experimentation, but
  it is not fully satisfactory (for instance, I don't want
  $ to be treated especially).  If there is agreement, then
  I will look into in in more details to design a better
  bison_quoting_style.

Attached:

- the (truncated) test-suite.log so that people can look at
  how this changes the messages

- the patch, also available on git as candidates/quotes

testsuite.log
Description: Binary data

0001-use-a-more-consistent-quoting-style.patch
Description: Binary data