bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Miscellaneous code readability improvements.


From: Joel E. Denny
Subject: Re: [PATCH] Miscellaneous code readability improvements.
Date: Wed, 19 Aug 2009 21:22:26 -0400 (EDT)
User-agent: Alpine 1.00 (DEB 882 2007-12-20)

On Wed, 19 Aug 2009, Akim Demaille wrote:

> >  \\(.|\n)   {
> > -    complain_at (*loc, _("unrecognized escape sequence: %s"), quote
> > (yytext));
> > -    STRING_GROW;
> > +    char const *p = yytext + 1;
> > +    if (*p == ' ')
> > +      p = "` '";
> > +    else
> > +      p = quotearg_style_mem (escape_quoting_style, p, 1);
> > +    complain_at (*loc, _("invalid character after \\-escape: %s"), p);
> 
> Is space the only "invisible" character not made visible by quote arg?

As far as I can tell, but I don't know if that's guaranteed for all 
locales.

> What
> about tabulation for instance?

quotearg checks isprint, which returns true for space but false for 
characters like tabs, so they become \t or \v.  So, to be safe for all 
locales, I suppose we need to quote any character in the intersection of 
isspace and isprint.  Unfortunately, I know very little about locales, so 
maybe this is overkill.

I also know very little about multibyte characters and wide characters.  
The Flex manual says `.' only matches single-byte characters, so that's 
all I'm going to worry about.  However, quotearg invokes iswprint instead 
of isprint for some single byte characters in multibyte locales.  I'm 
going to assume that, for all single-byte characters, iswprint and 
iswspace return the same results as isprint and isspace.  If that's true, 
it should be sufficient to merge the following patch into my previous 
patch.

I pushed the result to master, branch-2.5, and branch-2.4.2.

diff --git a/src/scan-gram.l b/src/scan-gram.l
index ac91e31..b431e24 100644
--- a/src/scan-gram.l
+++ b/src/scan-gram.l
@@ -37,6 +37,7 @@
 #include <src/reader.h>
 #include <src/uniqstr.h>
 
+#include <ctype.h>
 #include <mbswidth.h>
 #include <quote.h>
 
@@ -616,8 +617,12 @@ splice      (\\[ \f\t\v]*\n)*
   }
   \\(.|\n)     {
     char const *p = yytext + 1;
-    if (*p == ' ')
-      p = "` '";
+    char quoted_ws[] = "` '";
+    if (isspace (*p) && isprint (*p))
+      {
+        quoted_ws[1] = *p;
+        p = quoted_ws;
+      }
     else
       p = quotearg_style_mem (escape_quoting_style, p, 1);
     complain_at (*loc, _("invalid character after \\-escape: %s"), p);




reply via email to

[Prev in Thread] Current Thread [Next in Thread]