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

Le 30 janv. 2012 à 10:43, Akim Demaille a écrit :

> Here is a patch that does all this.  candidates/quotes is
> updated.
> 
> <0001-use-a-more-consistent-quoting-style.patch>

This does not work:

> --- a/bootstrap.conf
> +++ b/bootstrap.conf
> @@ -22,7 +22,7 @@ gnulib_modules='
>   gettext git-version-gen gitlog-to-changelog
>   gpl-3.0 hash inttypes isnan javacomp-script
>   javaexec-script ldexpl maintainer-makefile malloc-gnu mbschr mbsrchr
> -  mbswidth obstack perror pipe-posix quote quotearg realloc-posix
> +  mbswidth obstack perror pipe-posix quotearg realloc-posix
>   spawn-pipe stdbool stpcpy strdup-posix strerror strtoul strverscmp

> --- /dev/null
> +++ b/lib/quote.c
> @@ -0,0 +1,54 @@
> +/* quote.c - quote arguments for output
> +
> +   Copyright (C) 1998-2001, 2003, 2005-2006, 2009-2012 Free Software
> +   Foundation, Inc.

I meant to replace gnulib's quote module by a specific
implementation for Bison.  But some other gnulib module

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

On 02/02/2012 02:49 AM, Akim Demaille wrote:
> - submit changes to gnulib's quote so that the choice of the default
>  style is left to the actual user of gnulib, and stick to
>  gnulib's quote in Bison.

Sounds good to me; how would that work?

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

Le 2 févr. 2012 à 16:52, Paul Eggert a écrit :

> On 02/02/2012 02:49 AM, Akim Demaille wrote:
>> - submit changes to gnulib's quote so that the choice of the default
>> style is left to the actual user of gnulib, and stick to
>> gnulib's quote in Bison.
> 
> Sounds good to me; how would that work?

Hi Paul!

I'm not sure what the best way to do it yet.  But there
are really two different issues covered in my patch,
one being to rely on quote() more extensively to provide
the quotes (rather than leaving them in the format
strings), the other being addressing the test suite
issues on OS X.  I pushed the first part in both master
(see the attached patch) and in branch-2.5.

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

Le 8 févr. 2012 à 11:54, Akim Demaille a écrit :

> Le 2 févr. 2012 à 16:52, Paul Eggert a écrit :
> 
>> On 02/02/2012 02:49 AM, Akim Demaille wrote:
>>> - submit changes to gnulib's quote so that the choice of the default
>>> style is left to the actual user of gnulib, and stick to
>>> gnulib's quote in Bison.
>> 
>> Sounds good to me; how would that work?
> 
> Hi Paul!
> 
> I'm not sure what the best way to do it yet.

My problem is that there are several default styles used
in quote*.  There is DEFAULT_QUOTING_STYLE which does
not seem to be used (but maybe its purpose is to tell
the users of quotearg.h that 0 corresponds to
literal_quoting_style?).  There is locale_quote_style
which is hard coded in quote, and also there is
the default_quoting_style that can be modified by the
user.

So I don't know if you want quote to use the default
style (0), or to have its own style.  And actually, by
style I mean options.

The proposal below exposes a new quote_quoting_options,

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

On 02/08/2012 02:54 AM, Akim Demaille wrote:
> -        error (0, 0, _("missing operand after '%s'"), argv[argc - 1]);
> +        error (0, 0, _("%s: missing operand"), quotearg_colon (argv[argc - 1]));
>        else
> -        error (0, 0, _("extra operand '%s'"), argv[optind + 1]);
> +        error (0, 0, _("extra operand %s"), quote (argv[optind + 1]));

This doesn't seem consistent: shouldn't both use quotearg_colon,
or neither?

Otherwise I didn't see a problem, in a quick perusal.

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

Le 8 févr. 2012 à 17:32, Paul Eggert a écrit :

> On 02/08/2012 02:54 AM, Akim Demaille wrote:
>> -        error (0, 0, _("missing operand after '%s'"), argv[argc - 1]);
>> +        error (0, 0, _("%s: missing operand"), quotearg_colon (argv[argc - 1]));
>>       else
>> -        error (0, 0, _("extra operand '%s'"), argv[optind + 1]);
>> +        error (0, 0, _("extra operand %s"), quote (argv[optind + 1]));
> 
> This doesn't seem consistent: shouldn't both use quotearg_colon,
> or neither?

Hi Paul,

thanks for the review.  Actually, I am bugged by this
one: I can read easily

	--output: missing operand

but I don't read as easily

	output: extra operand

I would go for

	extra operand: output

for instance.  Is this what you mean?

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

On 02/08/2012 10:03 AM, Akim Demaille wrote:
> I would go for
> 
> 	extra operand: output
> 
> for instance.  Is this what you mean?

Sorry, I wasn't paying attention at all to the actual messages:
all I noticed was that quotearg_colon was used with one, and
quote with the other.  Now that you've drawn my attention to
what the messages actually mean, I agree that it's OK that
they're inconsistent (either with or without your latest
suggestion).

"check" table missing comment in bison.m4

Hi,

the "check" table (yycheck_[] in the Java incarnation) has
no comment in bison.m4's b4_integral_parser_tables_map which
leads to a misindentation in the generated Java scanner:

| [...]

| /* YYTABLE[YYPACT[STATE-NUM]].  What to do in state STATE-NUM.  If
|    positive, shift that token.  If negative, reduce the rule which
|    number is the opposite.  If YYTABLE_NINF, syntax error.  */
|   private static final byte yytable_[] = yytable_init();
|   private static final byte[] yytable_init()
|   {
| [...]
|   }

| private static final byte yycheck_[] = yycheck_init();
|   private static final byte[] yycheck_init()
|   {
| [...]
|   }

| /* STOS_[STATE-NUM] -- The (internal number of the) accessing
|    symbol of state STATE-NUM.  */
|   private static final byte yystos_[] = yystos_init();
|   private static final byte[] yystos_init()
|   {
| [...]

master: bootstrap doesn't clean up properly

Hi,

on a freshly cloned copy, after "git submodule update --init
&& ./bootstrap && ./configure && make && make check", "git
status" gives:

| # On branch master
| # Untracked files:
| #   (use "git add <file>..." to include in what will be committed)
| #
| #	build-aux/config.rpath~
| #	build-aux/snippet/
| #	lib/glthread/
| #	m4/codeset.m4~
| #	m4/fcntl-o.m4~
| #	m4/gettext.m4~
| #	m4/glibc2.m4
| #	m4/glibc2.m4~
| #	m4/glibc21.m4~
| #	m4/iconv.m4~
| #	m4/intdiv0.m4
| #	m4/intdiv0.m4~
| #	m4/intl.m4
| #	m4/intl.m4~
| #	m4/intldir.m4
| #	m4/intldir.m4~
| #	m4/intlmacosx.m4~
| #	m4/intmax.m4
| #	m4/intmax.m4~
| #	m4/inttypes-pri.m4~

Re: Java parser throws ArrayIndexOutOfBoundsException if first token leads to syntax error

I wrote:

> [...]
>   There is a test for syntax errors on the first token in
> tests/java.at, but apparently only in combination with an
> "error" token.  I hope fixing it is easier than isolating
> it was .

Nope.  Anyway, attached is the fix.  The test case is a bit
non-hackily lengthy, but I did not want to interlace it in
the existing suite and subtly break half of them in the
process.

Tim

From 415333112ba8224264d75164628ca1a173ffb514 Mon Sep 17 00:00:00 2001
From: Tim Landscheidt <tim <at> tim-landscheidt.de>
Date: Sun, 12 Feb 2012 01:29:41 +0000
Subject: [PATCH] Java: Fix syntax error handling without error token.

* data/lalr1.java (YYParser::parse): Here.
* tests/java.at: Add test case.
---
 data/lalr1.java |    2 +-
 tests/java.at   |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletions(-)

diff --git a/data/lalr1.java b/data/lalr1.java
index 57ff993..a40d6c2 100644