[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 10/11] quote consistently and make tests pass with new quotin
From: |
Akim Demaille |
Subject: |
Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib |
Date: |
Sat, 28 Jan 2012 14:06:21 +0100 |
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
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, (continued)
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/25
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Paul Eggert, 2012/01/25
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/26
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/26
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Paul Eggert, 2012/01/26
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/27
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Paul Eggert, 2012/01/27
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib,
Akim Demaille <=
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/28
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Paul Eggert, 2012/01/28
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/29
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Paul Eggert, 2012/01/29
- Re: [PATCH 10/11] quote consistently and make tests pass with new quoting from gnulib, Akim Demaille, 2012/01/30
- Re: locale_charset() on MacOS X, Bruno Haible, 2012/01/26
- Re: locale_charset() on MacOS X, Hans Aberg, 2012/01/27
[PATCH 02/11] maint: get gpl-3.0 from gnulib, Jim Meyering, 2012/01/18