ruby-core

Matz, Nobu:

I've just looked over your recent patches to parse.y for file encoding 
comments.  set_file_encoding is a nice piece of work!  What a sweet 
switch statement :-)

I notice, however, that you use strncasecmp in one part of the function 
but only check for lowercase letters in the switch.  This results in the 
strange behavior where this comment sets the encoding to UTF-8

     #CODING: utf-8

But this comment, with just one extra space, does not:

     # CODING: utf-8

The attached file "patch1" is a simple patch that fixes this problem.

The attached "patch2" is a larger patch that deletes all of the 
magic_comment code in the file because the new set_file_encoding 
function is general enough that it is no longer needed.  I'm in over my 
head here and this may be a completely inappropriate patch, especially 
since it deletes some ifdef RIPPER stuff.  But while I was making an 
aggressive patch like this, I took the liberty of changing the method 
name set_file_encoding to set_encoding_from_comment.

I hope these are helpful.

	David

Attachments (2)

patch1 (741 Bytes, text/x-diff)

Index: parse.y
===================================================================
--- parse.y	(revision 13687)
+++ parse.y	(working copy)
@@ -5693,12 +5693,12 @@
     for (;;) {
 	if (send - str <= 6) return;
 	switch (str[6]) {
-	  case 'c': str += 6; continue;
-	  case 'o': str += 5; continue;
-	  case 'd': str += 4; continue;
-	  case 'i': str += 3; continue;
-	  case 'n': str += 2; continue;
-	  case 'g': str += 1; continue;
+	case 'C': case 'c': str += 6; continue;
+	case 'O': case 'o': str += 5; continue;
+	case 'D': case 'd': str += 4; continue;
+	case 'I': case 'i': str += 3; continue;
+	case 'N': case 'n': str += 2; continue;
+	case 'G': case 'g': str += 1; continue;
 	  case '=': case ':':
 	    sep = 1;
 	    str += 6;

patch2 (4.85 KB, text/x-diff)

Index: parse.y
===================================================================
--- parse.y	(revision 13687)
+++ parse.y	(working copy)
@@ -5534,7 +5534,6 @@
 #endif
 }
 
-/* emacsen -*- hack */
 static void
 parser_set_encode(struct parser_params *parser, const char *name)
 {
@@ -5545,147 +5544,10 @@
     parser->enc = rb_enc_from_index(idx);
 }
 
-#ifndef RIPPER
-typedef void (*rb_magic_comment_setter_t)(struct parser_params *parser, const char *name, const char *val);
-
 static void
-magic_comment_encoding(struct parser_params *parser, const char *name, const char *val)
+set_encoding_from_comment(struct parser_params *parser,
+			  const char *str, const char *send)
 {
-    if (parser && parser->line_count != (parser->has_shebang ? 2 : 1))
-	return;
-    parser_set_encode(parser, val);
-}
-
-struct magic_comment {
-    const char *name;
-    rb_magic_comment_setter_t func;
-};
-
-static const struct magic_comment magic_comments[] = {
-    {"coding", magic_comment_encoding},
-    {"encoding", magic_comment_encoding},
-};
-#endif
-
-static const char *
-magic_comment_marker(const char *str, int len)
-{
-    int i = 2;
-
-    while (i < len) {
-	switch (str[i]) {
-	  case '-':
-	    if (str[i-1] == '*' && str[i-2] == '-') {
-		return str + i + 1;
-	    }
-	    i += 2;
-	    break;
-	  case '*':
-	    if (i + 1 >= len) return 0;
-	    if (str[i+1] != '-') {
-		i += 4;
-	    }
-	    else if (str[i-1] != '-') {
-		i += 2;
-	    }
-	    else {
-		return str + i + 2;
-	    }
-	    break;
-	  default:
-	    i += 3;
-	    break;
-	}
-    }
-    return 0;
-}
-
-static int
-parser_magic_comment(struct parser_params *parser, const char *str, int len)
-{
-    VALUE name = 0, val = 0;
-    const char *beg, *end, *vbeg, *vend;
-#define str_copy(_s, _p, _n) ((_s) \
-	? (rb_str_resize((_s), (_n)), \
-	   MEMCPY(RSTRING_PTR(_s), (_p), char, (_n)), (_s)) \
-	: ((_s) = STR_NEW((_p), (_n))))
-
-    if (len <= 7) return Qfalse;
-    if (!(beg = magic_comment_marker(str, len))) return Qfalse;
-    if (!(end = magic_comment_marker(beg, str + len - beg))) return Qfalse;
-    str = beg;
-    len = end - beg - 3;
-
-    /* %r"([^\\s\'\":;]+)\\s*:\\s*(\"(?:\\\\.|[^\"])*\"|[^\"\\s;]+)[\\s;]*" */
-    while (len > 0) {
-#ifndef RIPPER
-	const struct magic_comment *p = magic_comments;
-#endif
-	int n = 0;
-
-	for (; len > 0 && *str; str++, --len) {
-	    switch (*str) {
-	      case '\'': case '"': case ':': case ';':
-		continue;
-	    }
-	    if (!ISSPACE(*str)) break;
-	}
-	for (beg = str; len > 0; str++, --len) {
-	    switch (*str) {
-	      case '\'': case '"': case ':': case ';':
-		break;
-	      default:
-		if (ISSPACE(*str)) break;
-		continue;
-	    }
-	    break;
-	}
-	for (end = str; len > 0 && ISSPACE(*str); str++, --len);
-	if (!len) break;
-	if (*str != ':') continue;
-
-	do str++; while (--len > 0 && ISSPACE(*str));
-	if (!len) break;
-	if (*str == '"') {
-	    for (vbeg = ++str; --len > 0 && *str != '"'; str++) {
-		if (*str == '\\') {
-		    --len;
-		    ++str;
-		}
-	    }
-	    vend = str;
-	    if (len) {
-		--len;
-		++str;
-	    }
-	}
-	else {
-	    for (vbeg = str; len > 0 && *str != '"' && !ISSPACE(*str); --len, str++);
-	    vend = str;
-	}
-	while (len > 0 && (*str == ';' || ISSPACE(*str))) --len, str++;
-
-	n = end - beg;
-	str_copy(name, beg, n);
-#ifndef RIPPER
-	do {
-	    if (strncasecmp(p->name, RSTRING_PTR(name), n) == 0) {
-		str_copy(val, vbeg, vend - vbeg);
-		(*p->func)(parser, RSTRING_PTR(name), RSTRING_PTR(val));
-		break;
-	    }
-	} while (++p < magic_comments + sizeof(magic_comments) / sizeof(*p));
-#else
-	dispatch2(magic_comment, name, val);
-#endif
-    }
-
-    return Qtrue;
-}
-
-static void
-set_file_encoding(struct parser_params *parser, const char *str, const char *send)
-{
     int sep = 0;
     const char *beg = str;
     VALUE s;
@@ -5693,12 +5555,12 @@
     for (;;) {
 	if (send - str <= 6) return;
 	switch (str[6]) {
-	  case 'c': str += 6; continue;
-	  case 'o': str += 5; continue;
-	  case 'd': str += 4; continue;
-	  case 'i': str += 3; continue;
-	  case 'n': str += 2; continue;
-	  case 'g': str += 1; continue;
+	case 'C': case 'c': str += 6; continue;
+	case 'O': case 'o': str += 5; continue;
+	case 'D': case 'd': str += 4; continue;
+	case 'I': case 'i': str += 3; continue;
+	case 'N': case 'n': str += 2; continue;
+	case 'G': case 'g': str += 1; continue;
 	  case '=': case ':':
 	    sep = 1;
 	    str += 6;
@@ -5815,13 +5677,8 @@
 	goto retry;
 
       case '#':		/* it's a comment */
-	if (!parser->has_shebang || parser->line_count != 1) {
-	    /* no magic_comment in shebang line */
-	    if (!parser_magic_comment(parser, lex_p, lex_pend - lex_p)) {
-		if (parser->line_count == (parser->has_shebang ? 2 : 1)) {
-		    set_file_encoding(parser, lex_p, lex_pend);
-		}
-	    }
+	if (parser->line_count == (parser->has_shebang ? 2 : 1)) {
+	  set_encoding_from_comment(parser, lex_p, lex_pend);
 	}
 	lex_p = lex_pend;
 #ifdef RIPPER

Thread

Prev Next

In This Thread

Prev Next

[#12312] Need Japanese Help - VRuby & new One-Click Ruby Installer with patch 110 — "Curt Hibbs" <curt.hibbs@...>

[#12328] Dir.chdir patch for MS Windows — "Berger, Daniel" <Daniel.Berger@...>

[#12336] Re: gc.c -- possible logic error? — Brent Roman <brent@...>

[#12338] gc.c refactoring — Hugh Sasse <hgs@...>

[#12344] patch to implement Array.permutation — David Flanagan <david@...>

[#12346] a patch for Array.permutation and a generalization of Array.product — David Flanagan <david@...>

[#12351] ruby19 omits program name for missing files — Eric Hodel <drbrain@...7.net>

[#12352] system and Errno::ENOENT in 1.9 — Eric Hodel <drbrain@...7.net>

[#12371] Bug 6400: Time#+ and Time#- not honoring subclass — "Ron Leisti" <ron.leisti@...>

[#12372] Release compatibility/train — Prashant Srinivasan <Prashant.Srinivasan@...>

[#12375] some typos in doc/comments in repos/ruby/trunk — Eugene Ossintsev <eugoss@...>

[#12379] discrepancy between String#oct and String#hex — Ryan Davis <ryand-ruby@...>

[#12383] Include Rake in Ruby 1.9 — "NAKAMURA, Hiroshi" <nakahiro@...>

[#12401] tIDENTIFIER and is_local_id() — Thomas Enebo <Thomas.Enebo@...>

[#12404] ruby-1.9 build Sun-sparc-solaris2.9 — Hugh Sasse <hgs@...>

[#12411] rdoc patch — David Flanagan <david@...>

[#12412] REXML + RSS — Sean Russell <ser@...>

[#12414] Performance Issues with nil, true, false as Hash Keys (1.8.6) — =?ISO-8859-15?Q?Matthias_W=E4chter?= <matthias@...>

[#12418] Patch for configure.in — "Richard Brown" <rbrown@...>

[#12529] Re: Compile error for HEAD — "NARUSE, Yui" <naruse@...>

[#12533] Quick Enumerable#sort_by internals question — Daniel Berger <Daniel.Berger@...>

[#12537] Re: Why does def return nil? — "Jonas Pfenniger" <zimbatm@...>

[#12539] Ordered Hashes in 1.9? — Michael Neumann <mneumann@...>

[#12540] Re: Why does def return nil? — "Jonas Pfenniger" <zimbatm@...>

[#12547] Bug in win32ole — "Waters, Chris" <cwaters@...>

[#12550] to let Ruby 1.9 to process signals while blocked in ext/readline — Brent Roman <brent@...>

[#12564] Re: net/http.rb, patch, possible bug, test problem — Hugh Sasse <hgs@...>

[#12566] Revised net/http patch — Hugh Sasse <hgs@...>

[#12568] $" and require — "Tim Morgan" <tmorgan99@...>

[#12573] File::Stat#grpowned? is wrong — Bertram Scharpf <lists@...>

[#12575] Marshal.dump doesn't dump string encoding — David Flanagan <david@...>

[#12578] Possible memory leak in ruby-1.8.6-p110?? — "M. Edward (Ed) Borasky" <znmeb@...>

[#12579] iconv enhancement in Ruby 1.9 — "Eugene Ossintsev" <eugoss@...>

[#12581] ruby.exe exit code 255 — "Drew Sears" <dsears@...>

[#12584] Broken Ruby/ODBC due rbconfig COMMON_MACROS — "Luis Lavena" <luislavena@...>

[#12587] Confusion about arities — Charles Oliver Nutter <charles.nutter@...>

[#12588] MatchData#select rdoc and arity incorrect — Charles Oliver Nutter <charles.nutter@...>

[#12598] adds __file__ and __line__ methods to Methods and Procs — Brent Roman <brent@...>

[#12599] private method dispatch changes dropped from 1.9? — David Flanagan <david@...>

[#12610] Re: Release compatibility/train — Yukihiro Matsumoto <matz@...>

[#12615] Re: Why does def return nil? — Brent Roman <brent@...>

[#12617] Question about heap_slots in gc.c — Hongli Lai <h.lai@...>

[#12618] StringIO is not IO? — Hongli Lai <h.lai@...>

[#12623] Behaviour of String Objects - Why Not "Each" for Characters and Enumerable? — =?ISO-8859-15?Q?Wolfgang_N=E1dasi-Donner?= <ed.odanow@...>

[#12629] file encoding comments and a patch to parse.y — David Flanagan <david@...>

[#12632] Defining unicode methods — "Daniel Berger" <djberg96@...>

[#12636] Proc.yield method — David Flanagan <david@...>

[#12638] New lambda -> syntax — David Flanagan <david@...>

[#12639] test problem after compiling — Martin Duerst <duerst@...>

[#12664] minor rdoc patch to encoding.c — David Flanagan <david@...>

[#12665] Another encoding.c patch — David Flanagan <david@...>

[#12666] encoding.c bug — David Flanagan <david@...>

[#12670] Bug in Numeric#divmod — "Dirk Traulsen" <dirk.traulsen@...>

[#12675] Re: Import gem to Ruby 1.9 — Brent Roman <brent@...>

[#12676] Minor Array.shuffle patch — David Flanagan <david@...>

[#12679] documentation patch for enum.c — David Flanagan <david@...>

[#12681] Unicode: Progress? — murphy <murphy@...>

[#12693] retry: revised 1.9 http patch — Hugh Sasse <hgs@...>

[#12695] New Rails plugin 'Liftoff' is available. — "Ben Tompkins" <nbitspoken@...>

[#12697] Range.first is incompatible with Enumerable.first — David Flanagan <david@...>

[#12703] Long encoding names with -K and bad error message — David Flanagan <david@...>

[#12705] A couple of bugs? — "John Lam (DLR)" <jflam@...>

[#12706] Re: A couple of bugs? — "Gavin Kistner" <gavin.kistner@...>

[#12710] enum.c patch: fixes Enumerable.cycle and rdoc bugs — David Flanagan <david@...>

[#12714] Re: A couple of bugs? — "Gavin Kistner" <gavin.kistner@...>

[#12750] Re: [OT] Re: Should *most* memory be release back to the system? — "M. Edward (Ed) Borasky" <znmeb@...>

[#12754] Improving 'syntax error, unexpected $end, expecting kEND'? — Hugh Sasse <hgs@...>

[#12757] Re: Unicode: Progress?? — Brent Roman <brent@...>

[#12758] Encoding::primary_encoding — David Flanagan <david@...>

[#12759] Reading UTF-8 Data contains BOM — =?ISO-8859-15?Q?Wolfgang_N=E1dasi-Donner?= <ed.odanow@...>

[#12764] SystemStackError 1.8.6 (stack only 25 frames) — Dan Janowski <danj@3skel.com>

[#12767] \u escapes in string literals: proof of concept implementation — David Flanagan <david@...>

[#12776] Clarification of Ruby release plans? 1.8.7? 1.9.1? 2.0? — Lucas Nussbaum <lucas@...>

[#12779] Re: Unicode: Progress?? — Brent Roman <brent@...>

[#12780] Re: Unicode: Progress?? — Brent Roman <brent@...>

[#12787] How to specify in Ruby 1.9 the expected file encoding — =?ISO-8859-15?Q?Wolfgang_N=E1dasi-Donner?= <ed.odanow@...>

[#12795] patch for String.concat — David Flanagan <david@...>

[#12825] clarification of ruby libraries installation paths? — Lucas Nussbaum <lucas@...>

[#12834] Bug Tracker Access Request — James Edward Gray II <james@...>

[#12847] make test-all — "Florian Ebeling" <florian.ebeling@...>

[#12598] adds file and line methods to Methods and Procs — Brent Roman <brent@...>

[#12750] Re: [OT] Re: Should most memory be release back to the system? — "M. Edward (Ed) Borasky" <znmeb@...>