ruby-core

Hi,

Attached is a patch that implements "Encoding::default_internal". The aim  
of this patch is to allow a Ruby programmer to specify a default encoding  
that all IO (and I hope other methods) will return strings in, so that you  
can deal with the strings without worrying (too much) about encoding  
compatibility.

In doing this patch I have attempted to take everyone's input into account.

I have spent quite a bit of time on this (mainly thinking about the  
features that will make it useful for everyone), so I hope that it is  
accepted, even if it is a bit late!

A summary of what this patch does:

1) Extends the "-E" command line flag to accept either "ext_enc",  
"ext_enc:int_enc" or ":int_enc". If the form "ext_enc:int_enc" is used,  
the value of BOTH default_external & default_internal is set. If the form  
":int_enc" is used, only the default_internal is set - the  
default_external is left at it's default value. Similarly if the form  
"ext_enc" is used - default_internal is left at it's default value.

2) Extended the "magic comment" feature to also look for  
"internal_encoding: XXXX". If this is found in the MAIN ruby file only,  
both the source encoding AND the default_internal are set to the given  
encoding. If found in an included source file (eg: library), it is treated  
the same way as "encoding:" and sets the source encoding only. (You  
shouldn't use it in a library anyhow).

3) The above 2 features give the Ruby programmer 2 ways of specifying the  
value of default_internal in their main program:
	- By using "internal_encoding: XXX" magic comment, or
	- By using -E:XXX in the ruby "shebang" on the first line (note the ":"  
before the XXX to indicate "internal encoding").
The second form allows the programmer to specify a different value for  
"default_internal" than the source encoding (not sure if that is  
particularly useful).
Use of both of these is ignored when not in the main program, so that  
"default_internal" can only be set once and only under the control of the  
programmer or user.
Note: If the default_internal is specified on the ruby command line, it  
overrides both of these.

4) If for some misguided reason default_internal is set to US-ASCII, it  
actually sets itself to UTF-8, as this is much more likely to cope better.

5) The new method Encoding#default_internal works just like  
Encoding#default_external - it is read-only. If it is not set, it's value  
is nil. The is no "default_internal=" method.

6) When setting the encodings of an IO, if no internal encoding is  
specified, the default_internal value will be used if it is set. If it is  
not set, there is no transcoding.

7) There is one exception to the use of "default_internal" when opening a  
file with an external encoding of ASCII-8BIT (aka BINARY). In this case  
"default_internal" is ignored, and data returned untranscoded because the  
user probably wants to do byte or bit processing rather than character.

8) The "int_enc" passed in the file mode or to IO#set_encoding may also be  
"-". This will force the internal encoding to nil for the file, so no  
transcoding is done on input.
For example: open(file, "r:EUC-JP:-") or f.set_encoding("EUC-JP", "-").
This gives the programmer the ability to prevent transcoding even if  
"default_internal" is set.



Notes:

- When default_internal is not set Ruby works exactly as it does now in  
1.9.0

- "Unicode whiners" (not my expression!) should probably set  
"default_internal" to UTF-8.

- "default_internal" does *not* guarantee that all strings in your program  
will be in that encoding. What it does (currently) is work on IO when  
reading files or sockets etc. You can still use String#encode,  
String#force_encoding etc to change them. It would  be nice to actually  
have a way of *forcing* all strings (and Regexps) to be a particular  
encoding in order to guarantee no "encoding compatibility" problems.  
However this is quite a bit more complicated to implement. So when writing  
libraries & modules, you may still have to check the encoding of strings  
passed to your methods.

- Using the "internal_encoding:" magic comment in your main program is the  
preferred way of setting default_internal. This is because both your  
source encoding and data read from files will be the same encoding, so at  
least your program's string literals should match too.

- there are probably other libraries that should be patched to support  
"default_internal" to ensure that strings are returned to the user in that  
encoding. However ones like OpenURI *should* just work, because of the way  
"default_internal" is supported transparently in the IO class.

- [I have not implemented this, because it would probably be rejected.  
However I'll say it because I still think it is a good idea] I think if a  
file is open with "b" mode, it should default to ASCII-8BIT, no  
transcoding. This would probably get rid of the need for "bin_read" too.


Implementation notes:

- encoding.c/encoding.h - added functions to get & set "default_internal"
- parse.y - added code to support the "internal_encoding" magic comment
- ruby.c - added code to extend -E/--encoding option
- io.c - added code to support default_internal. Note that I almost  
completely rewrote parse_mode_enc(). The other main mod is a new function  
rb_io_ext_int_to_encs() which converts the external & internal encodings  
to values for "enc" & "enc2" in convconfig_t. It is shame that "enc" &  
"enc2" don't map directly to "internal" and "external" - it would have  
made the code much simpler!
- Testing: not extensively tested. Passes "make test". Some "make  
test-all" tests fail (esp rdoc), but I think that they may be due to other  
issues (not certain).

I'm sure you'll tell me if I have broken anything!

Cheers
Mike

Attachments (1)

default_internal.pat (14.2 KB, text/x-diff)

Index: encoding.c
===================================================================
--- encoding.c	(revision 19561)
+++ encoding.c	(working copy)
@@ -1027,8 +1027,57 @@
     default_external = 0;
 }
 
+/* -2 => not yet set, -1 => nil */
+static int default_internal_index = -2;
+static rb_encoding *default_internal;
+
+rb_encoding *
+rb_default_internal_encoding(void)
+{
+    if (!default_internal && default_internal_index >= 0) {
+	default_internal = rb_enc_from_index(default_internal_index);
+    }
+    return default_internal;
+}
+
+VALUE
+rb_enc_default_internal(void)
+{
+    /* Note: These functions cope with default_internal not being set */
+    return rb_enc_from_encoding(rb_default_internal_encoding());
+}
+
 /*
  * call-seq:
+ *   Encoding.default_internal => enc
+ *
+ * Returns default internal encoding.
+ *
+ * It is initialized by the source internal_encoding or -E option,
+ * and can't be modified after that.
+ */
+static VALUE
+get_default_internal(VALUE klass)
+{
+    return rb_enc_default_internal();
+}
+
+void
+rb_enc_set_default_internal(VALUE encoding)
+{
+    if (default_internal_index != -2)
+	/* Already set */
+	return;
+    default_internal_index = encoding == Qnil ?
+				-1 :rb_enc_to_index(rb_to_encoding(encoding));
+    /* Convert US-ASCII => UTF-8 */
+    if (default_internal_index == rb_usascii_encindex())
+	default_internal_index = rb_utf8_encindex();
+    default_internal = 0;
+}
+
+/*
+ * call-seq:
  *   Encoding.locale_charmap => string
  *
  * Returns the locale charmap name.
@@ -1212,6 +1261,7 @@
     rb_define_singleton_method(rb_cEncoding, "_load", enc_load, 1);
 
     rb_define_singleton_method(rb_cEncoding, "default_external", get_default_external, 0);
+    rb_define_singleton_method(rb_cEncoding, "default_internal", get_default_internal, 0);
     rb_define_singleton_method(rb_cEncoding, "locale_charmap", rb_locale_charmap, 0);
 
     list = rb_ary_new2(enc_table.count);
Index: include/ruby/encoding.h
===================================================================
--- include/ruby/encoding.h	(revision 19561)
+++ include/ruby/encoding.h	(working copy)
@@ -168,11 +168,14 @@
 rb_encoding *rb_locale_encoding(void);
 rb_encoding *rb_filesystem_encoding(void);
 rb_encoding *rb_default_external_encoding(void);
+rb_encoding *rb_default_internal_encoding(void);
 int rb_ascii8bit_encindex(void);
 int rb_utf8_encindex(void);
 int rb_usascii_encindex(void);
 VALUE rb_enc_default_external(void);
+VALUE rb_enc_default_internal(void);
 void rb_enc_set_default_external(VALUE encoding);
+void rb_enc_set_default_internal(VALUE encoding);
 VALUE rb_locale_charmap(VALUE klass);
 long rb_memsearch(const void*,long,const void*,long,rb_encoding*);
 
Index: io.c
===================================================================
--- io.c	(revision 19561)
+++ io.c	(working copy)
@@ -2183,10 +2183,8 @@
 	}
 	newline = (unsigned char)rsptr[rslen - 1];
 
-        if (fptr->encs.enc2)
-            enc = fptr->encs.enc;
-        else
-            enc = io_input_encoding(fptr);
+	/* MS - Optimisation */
+        enc = io_read_encoding(fptr);
 	while ((c = appendline(fptr, newline, &str, &limit)) != EOF) {
             const char *s, *p, *pp, *e;
 
@@ -3746,52 +3744,87 @@
     return NULL;		/* not reached */
 }
 
+/*
+ * Convert external/internal encodings to enc/enc2
+ * NULL => use default encoding
+ * Qnil => no encoding specified (internal only)
+ */
 static void
+rb_io_ext_int_to_encs(rb_encoding *ext, rb_encoding *intern, rb_encoding **enc, rb_encoding **enc2)
+{
+    int default_ext = 0;
+
+    if (ext == NULL) {
+	ext = rb_default_external_encoding();
+	default_ext = 1;
+    }
+    if (intern == NULL && ext != rb_ascii8bit_encoding())
+	/* If external is ASCII-8BIT, no default transcoding */
+	intern = rb_default_internal_encoding();
+    if (intern == NULL || intern == (rb_encoding *)Qnil || intern == ext) {
+	/* No internal encoding => use external + no transcoding */
+	*enc = default_ext ? NULL : ext;
+	*enc2 = NULL;
+    }
+    else {
+	*enc = intern;
+	*enc2 = ext;
+    }
+}
+
+static void
 parse_mode_enc(const char *estr, rb_encoding **enc_p, rb_encoding **enc2_p)
 {
-    const char *p0, *p1;
-    char *enc2name;
+    const char *p;
+    char encname[ENCODING_MAXNAMELEN+1];
     int idx, idx2;
+    rb_encoding *ext_enc, *int_enc;
 
-    /* parse estr as "enc" or "enc2:enc" */
+    /* parse estr as "enc" or "enc2:enc" or "enc:-" */
 
-    *enc_p = 0;
-    *enc2_p = 0;
+    p = strrchr(estr, ':');
+    if (p) {
+	int len = (p++) - estr;
+	if (len == 0 || len > ENCODING_MAXNAMELEN)
+	    idx = -1;
+	else {
+	    memcpy(encname, estr, len);
+	    encname[len] = '\0';
+	    estr = encname;
+	    idx = rb_enc_find_index(encname);
+	}
+    }
+    else
+	idx = rb_enc_find_index(estr);
 
-    p0 = strrchr(estr, ':');
-    if (!p0) p1 = estr;
-    else     p1 = p0 + 1;
-    idx = rb_enc_find_index(p1);
-    if (idx >= 0) {
-	*enc_p = rb_enc_from_index(idx);
-    }
+    if (idx >= 0)
+	ext_enc = rb_enc_from_index(idx);
     else {
-	rb_warn("Unsupported encoding %s ignored", p1);
+	if (idx != -2)
+	    rb_warn("Unsupported encoding %s ignored", estr);
+	ext_enc = NULL;
     }
 
-    if (*enc_p && p0) {
-	int n = p0 - estr;
-	if (n > ENCODING_MAXNAMELEN) {
-	    idx2 = -1;
+    int_enc = NULL;
+    if (p) {
+	if (*p == '-' && *(p+1) == '\0') {
+	    /* Special case - "-" => no transcoding */
+	    int_enc = (rb_encoding *)Qnil;
 	}
 	else {
-	    enc2name = ALLOCA_N(char, n+1);
-	    memcpy(enc2name, estr, n);
-	    enc2name[n] = '\0';
-	    estr = enc2name;
-	    idx2 = rb_enc_find_index(enc2name);
+	    idx2 = rb_enc_find_index(p);
+	    if (idx2 < 0)
+		rb_warn("Unsupported encoding %s ignored", p);
+	    else if (idx2 == idx) {
+		rb_warn("Ignoring internal encoding %s: it is identical to external encoding %s", p, estr);
+		int_enc = (rb_encoding *)Qnil;
+	    }
+	    else
+		int_enc = rb_enc_from_index(idx2);
 	}
-	if (idx2 < 0) {
-	    rb_warn("Unsupported encoding %.*s ignored", n, estr);
-	}
-	else if (idx2 == idx) {
-	    rb_warn("Ignoring internal encoding %.*s: it is identical to external encoding %s",
-		    n, estr, p1);
-	}
-	else {
-	    *enc2_p = rb_enc_from_index(idx2);
-	}
     }
+
+    rb_io_ext_int_to_encs(ext_enc, int_enc, enc_p, enc2_p);
 }
 
 static void
@@ -3827,28 +3860,32 @@
     }
     if (!NIL_P(extenc)) {
 	rb_encoding *extencoding = rb_to_encoding(extenc);
+	rb_encoding *intencoding = NULL;
         extracted = 1;
-        *enc_p = 0;
-        *enc2_p = 0;
 	if (!NIL_P(encoding)) {
 	    rb_warn("Ignoring encoding parameter '%s': external_encoding is used",
 		    RSTRING_PTR(encoding));
 	}
 	if (!NIL_P(intenc)) {
-	    rb_encoding *intencoding = rb_to_encoding(intenc);
+	    if (!NIL_P(encoding = rb_check_string_type(intenc))) {
+		char *p = StringValueCStr(encoding);
+		if (*p == '-' && *(p+1) == '\0') {
+		    /* Special case - "-" => no transcoding */
+		    intencoding = (rb_encoding *)Qnil;
+		}
+		else
+		    intencoding = rb_to_encoding(intenc);
+	    }
+	    else
+		intencoding = rb_to_encoding(intenc);
 	    if (extencoding == intencoding) {
 		rb_warn("Ignoring internal encoding '%s': it is identical to external encoding '%s'",
 			RSTRING_PTR(rb_inspect(intenc)),
 			RSTRING_PTR(rb_inspect(extenc)));
+		intencoding = (rb_encoding *)Qnil;
 	    }
-	    else {
-		*enc_p = intencoding;
-                *enc2_p = extencoding;
-	    }
 	}
-        else {
-            *enc_p = extencoding;
-        }
+	rb_io_ext_int_to_encs(extencoding, intencoding, enc_p, enc2_p);
     }
     else {
 	if (!NIL_P(intenc)) {
@@ -3888,8 +3925,8 @@
 
     vmode = *vmode_p;
 
-    enc = NULL;
-    enc2 = NULL;
+    /* Set to defaults */
+    rb_io_ext_int_to_encs(NULL, NULL, &enc, &enc2);
 
     if (NIL_P(vmode)) {
         fmode = FMODE_READABLE;
@@ -4076,8 +4113,8 @@
     rb_io_t *fptr;
     convconfig_t cc;
     if (!convconfig) {
-        cc.enc = NULL;
-        cc.enc2 = NULL;
+	/* Set to default encodings */
+	rb_io_ext_int_to_encs(NULL, NULL, &cc.enc, &cc.enc2);
         cc.ecflags = 0;
         cc.ecopts = Qnil;
         convconfig = &cc;
@@ -4105,8 +4142,8 @@
         parse_mode_enc(p+1, &convconfig.enc, &convconfig.enc2);
     }
     else {
-        convconfig.enc = NULL;
-        convconfig.enc2 = NULL;
+	/* Set to default encodings */
+	rb_io_ext_int_to_encs(NULL, NULL, &convconfig.enc, &convconfig.enc2);
         convconfig.ecflags = 0;
         convconfig.ecopts = Qnil;
     }
@@ -6677,29 +6714,40 @@
 {
     rb_encoding *enc, *enc2;
     int ecflags;
-    VALUE ecopts;
+    VALUE ecopts, tmp;
 
     if (!NIL_P(v2)) {
 	enc2 = rb_to_encoding(v1);
-	enc = rb_to_encoding(v2);
+	tmp = rb_check_string_type(v2);
+	if (!NIL_P(tmp)) {
+	    char *p = StringValueCStr(tmp);
+	    if (*p == '-' && *(p+1) == '\0') {
+		/* Special case - "-" => no transcoding */
+		enc = enc2;
+		enc2 = NULL;
+	    }
+	    else
+		enc = rb_to_encoding(v2);
+	}
+	else
+	    enc = rb_to_encoding(v2);
         ecflags = rb_econv_prepare_opts(opt, &ecopts);
     }
     else {
 	if (NIL_P(v1)) {
-	    enc = NULL;
-	    enc2 = NULL;
+	    /* Set to default encodings */
+	    rb_io_ext_int_to_encs(NULL, NULL, &enc, &enc2);
             ecflags = 0;
             ecopts = Qnil;
 	}
 	else {
-	    VALUE tmp = rb_check_string_type(v1);
+	    tmp = rb_check_string_type(v1);
 	    if (!NIL_P(tmp)) {
                 parse_mode_enc(StringValueCStr(tmp), &enc, &enc2);
                 ecflags = rb_econv_prepare_opts(opt, &ecopts);
 	    }
 	    else {
-		enc = rb_to_encoding(v1);
-		enc2 = NULL;
+		rb_io_ext_int_to_encs(rb_to_encoding(v1), NULL, &enc, &enc2);
                 ecflags = 0;
                 ecopts = Qnil;
 	    }
Index: parse.y
===================================================================
--- parse.y	(revision 19561)
+++ parse.y	(working copy)
@@ -6068,6 +6068,8 @@
     if (parser->line_count != (parser->has_shebang ? 2 : 1))
 	return;
     parser_set_encode(parser, val);
+    if (strcmp(name, "internal_encoding") == 0)
+	rb_enc_set_default_internal(rb_enc_from_encoding(parser->enc));
 }
 
 struct magic_comment {
@@ -6079,6 +6081,7 @@
 static const struct magic_comment magic_comments[] = {
     {"coding", magic_comment_encoding, parser_encode_length},
     {"encoding", magic_comment_encoding, parser_encode_length},
+    {"internal_encoding", magic_comment_encoding, parser_encode_length},
 };
 #endif
 
@@ -6207,6 +6210,8 @@
 {
     int sep = 0;
     const char *beg = str;
+    const char *name;
+    int name_len;
     VALUE s;
 
     for (;;) {
@@ -6229,6 +6234,11 @@
 	}
 	if (STRNCASECMP(str-6, "coding", 6) == 0) break;
     }
+    /* Search for the start of the keyword */
+    for (name = str-6; name >= beg; name--)
+	if (!ISALNUM(*name) && *name != '_')
+	    break;
+    name_len = str - ++name;
     for (;;) {
 	do {
 	    if (++str >= send) return;
@@ -6243,6 +6253,8 @@
     s = rb_str_new(beg, parser_encode_length(parser, beg, str - beg));
     parser_set_encode(parser, RSTRING_PTR(s));
     rb_str_resize(s, 0);
+    if (name_len == 17 && STRNCASECMP(name, "internal_encoding", 17) == 0)
+	rb_enc_set_default_internal(rb_enc_from_encoding(parser->enc));
 }
 
 static void
Index: ruby.c
===================================================================
--- ruby.c	(revision 19561)
+++ ruby.c	(working copy)
@@ -94,7 +94,7 @@
 	    VALUE name;
 	    int index;
 	} enc;
-    } src, ext;
+    } src, ext, intern;
     VALUE req_list;
 };
 
@@ -869,6 +869,7 @@
 		ruby_each_words(s, disable_option, &opt->disable);
 	    }
 	    else if (strncmp("encoding", s, n = 8) == 0 && (!s[n] || s[n] == '=')) {
+		char *p;
 		s += n;
 		if (!*s++) {
 		  next_encoding:
@@ -877,7 +878,15 @@
 		    }
 		}
 	      encoding:
-		opt->ext.enc.name = rb_str_new2(s);
+		p = strchr(s, ':');
+		if (p) {
+		    if (p > s)
+			opt->ext.enc.name = rb_str_new(s, p-s);
+		    if (*++p)
+			opt->intern.enc.name = rb_str_new2(p);
+		}
+		else    
+		    opt->ext.enc.name = rb_str_new2(s);
 	    }
 	    else if (strcmp("version", s) == 0)
 		opt->version = 1;
@@ -980,6 +989,7 @@
 	rb_safe_level() == 0 && (s = getenv("RUBYOPT"))) {
 	VALUE src_enc_name = opt->src.enc.name;
 	VALUE ext_enc_name = opt->ext.enc.name;
+	VALUE int_enc_name = opt->intern.enc.name;
 
 	while (ISSPACE(*s))
 	    s++;
@@ -1019,6 +1029,8 @@
 	    opt->src.enc.name = src_enc_name;
 	if (ext_enc_name)
 	    opt->ext.enc.name = ext_enc_name;
+	if (int_enc_name)
+	    opt->intern.enc.name = int_enc_name;
     }
 
     if (opt->version) {
@@ -1087,6 +1099,9 @@
     if (opt->ext.enc.name != 0) {
 	opt->ext.enc.index = opt_enc_index(opt->ext.enc.name);
     }
+    if (opt->intern.enc.name != 0) {
+	opt->intern.enc.index = opt_enc_index(opt->intern.enc.name);
+    }
     if (opt->src.enc.name != 0) {
 	opt->src.enc.index = opt_enc_index(opt->src.enc.name);
 	src_encoding_index = opt->src.enc.index;
@@ -1098,6 +1113,11 @@
 	enc = lenc;
     }
     rb_enc_set_default_external(rb_enc_from_encoding(enc));
+    if (opt->intern.enc.index >= 0) {
+	enc = rb_enc_from_index(opt->intern.enc.index);
+	rb_enc_set_default_internal(rb_enc_from_encoding(enc));
+	opt->intern.enc.index = -1;
+    }
 
     rb_set_safe_level_force(safe);
     if (opt->e_script) {
@@ -1119,6 +1139,15 @@
 	tree = load_file(parser, opt->script, 1, opt);
     }
 
+    if (opt->intern.enc.index >= 0) {
+	/* Set in the shebang line */
+	enc = rb_enc_from_index(opt->intern.enc.index);
+	rb_enc_set_default_internal(rb_enc_from_encoding(enc));
+    }
+    else
+	/* Freeze default_internal */
+	rb_enc_set_default_internal(Qnil);
+
     if (!tree) return Qfalse;
 
     process_sflag(opt);
@@ -1189,6 +1218,7 @@
 	char *p;
 	int no_src_enc = !opt->src.enc.name;
 	int no_ext_enc = !opt->ext.enc.name;
+	int no_int_enc = !opt->intern.enc.name;
 
 	enc = rb_usascii_encoding();
 	rb_funcall(f, rb_intern("set_encoding"), 1, rb_enc_from_encoding(enc));
@@ -1275,6 +1305,9 @@
 	    if (no_ext_enc && opt->ext.enc.name) {
 		opt->ext.enc.index = opt_enc_index(opt->ext.enc.name);
 	    }
+	    if (no_int_enc && opt->intern.enc.name) {
+		opt->intern.enc.index = opt_enc_index(opt->intern.enc.name);
+	    }
 	}
 	else if (!NIL_P(c)) {
 	    rb_io_ungetbyte(f, c);
@@ -1538,6 +1571,7 @@
     args.argv = argv;
     args.opt = cmdline_options_init(&opt);
     opt.ext.enc.index = -1;
+    opt.intern.enc.index = -1;
     tree = (NODE *)rb_vm_call_cfunc(rb_vm_top_self(),
 				    process_options, (VALUE)&args,
 				    0, rb_progname);

Thread

Prev Next

In This Thread

Prev Next