Feature #973

EncDet again

Added by Yuki Sonoda about 3 years ago. Updated 2 days ago.

[ruby-dev:37679]
Status:Assigned Start date:
Priority:Normal Due date:
Assignee:Yukihiro Matsumoto % Done:

0%

Category:-
Target version:2.0.0

Description

Yuguiです。

ZnZさんの日記(http://znz.s1.xrea.com/t/?date=20090102#c01 )経由でかつて
のEncDetライブラリの議論[ruby-dev:33628]を思い出しました。

さて、現在私が知る限りRDocとERBとIRBがそれぞれ独自にマジックコメントを解
釈してファイルを開く機能を実装しています。この重複具合は何らかの共通化の
必要性を示しているのではないかと思います。

前の議論ではファイル名で主に意見が一致せずに発散してしまったようです。
  encdet.rb <-> encoding/detector.rb

私はIOへの機能追加が良いのではないかと思いました。
  io/encdet.rb
  IO::magic_open(*args)  -> 内部でIO::openを呼び出し

実装を調整しないとランダムアクセスできないIOでは困るわけですが。

-- 
Yugui <yugui@yugui.jp>
http://yugui.jp
私は私をDumpする

History

Updated by Koichi Sasada about 3 years ago

  • Assignee set to Yukihiro Matsumoto
  • Priority changed from Low to Normal
  • Target version set to 1.9.2

Updated by Yukihiro Matsumoto about 3 years ago

まつもと ゆきひろです

In message "Re: [ruby-dev:37679] [FEATURE:trunk] EncDet again"
    on Sat, 3 Jan 2009 19:21:37 +0900, "Yugui (Yuki Sonoda)" <yugui@yugui.jp> writes:

|私はIOへの機能追加が良いのではないかと思いました。
|  io/encdet.rb

encはencodingの省略として(私には)認知できますが、detをdetect
の省略形として認識するのにはやや困難です。ただ、どうしてもダ
メというほどではないです。

|  IO::magic_open(*args)  -> 内部でIO::openを呼び出し

反対。magicでもmagicalでも名前が「魔法」すぎてなにをするのか
まったくわかりません。

Updated by Yui NARUSE about 3 years ago

成瀬です。

Yukihiro Matsumoto wrote:
> まつもと ゆきひろです
> 
> In message "Re: [ruby-dev:37679] [FEATURE:trunk] EncDet again"
>     on Sat, 3 Jan 2009 19:21:37 +0900, "Yugui (Yuki Sonoda)" <yugui@yugui.jp> writes:
> 
> |私はIOへの機能追加が良いのではないかと思いました。
> |  io/encdet.rb
> 
> encはencodingの省略として(私には)認知できますが、detをdetect
> の省略形として認識するのにはやや困難です。ただ、どうしてもダ
> メというほどではないです。

いっそ prelude に入れてしまいませんか。
そうすればライブラリ名に関しては悩む必要がなくなります。

> |  IO::magic_open(*args)  -> 内部でIO::openを呼び出し
> 
> 反対。magicでもmagicalでも名前が「魔法」すぎてなにをするのか
> まったくわかりません。

素直に IO::detect_open あたりは。

-- 
NARUSE, Yui  <naruse@airemix.jp>

Updated by Koichi Sasada about 3 years ago

 ささだです.

NARUSE, Yui wrote::
> いっそ prelude に入れてしまいませんか。
> そうすればライブラリ名に関しては悩む必要がなくなります。

 そんなに使うものなんですか?

-- 
// SASADA Koichi at atdot dot net

Updated by Kazuhiro NISHIYAMA about 2 years ago

Pythonだとchardetという名前のライブラリがあるようなので、
encdetでも良さそうな気がしますが、どうでしょうか?

http://chardet.feedparser.org/

Updated by Kouhei Sutou about 2 years ago

須藤です。

In <4b0d4b0e62b03_8c37c0cb6c133a@redmine.ruby-lang.org>
  "[ruby-dev:39775] [Feature #973] EncDet again" on Thu, 26 Nov 2009 00:19:42 +0900,
  Kazuhiro NISHIYAMA <redmine@ruby-lang.org> wrote:

> Pythonだとchardetという名前のライブラリがあるようなので、
> encdetでも良さそうな気がしますが、どうでしょうか?
> 
> http://chardet.feedparser.org/

わざわざわかりにくい名前を真似する必要はないと思います。
(*私には*わかりにくいです。)

Updated by Yui NARUSE about 2 years ago

chardetはアウトだと思います、"det"じゃなくて"char"の方が。
detの方も諸手を挙げて賛成というわけではありません。

しかし、現状rdoc絡みやerb周りなど、EncDetを再発明しようとした挙句に失敗してしまった例が散見されており、
そろそろこのライブラリは標準添付しないと悪しき遺産を残しかねないと憂慮しています。

言い換えると、このライブラリの用途は様々な場面で必要である一方、実装が意外と難しいので、
適切なライブラリを標準添付で提供しなければならないと思っています。
たとえ名前で合意がつかなかったとしても、yuguiさん判断で名前を決定し、添付するべきであろうと。

で、わたしは encdet でもいいと思っています。
なぜなら、これは使われるライブラリであり、使ってればどうせ慣れるから。

Updated by Akinori MUSHA about 2 years ago

ライブラリ名は encoding でいいんじゃないですか?

将来ほかにもEncoding絡みの追加ライブラリの必要が出てきたら、
encoding/detect などに移して encoding はよく使いそうな encoding/* をすべて require
(あるいはautoload)するようにすれば互換性を保てます。

またクラス名も、実装は Encoding::Detector などわかりやすい名前の下で行いつつ、
APIは(少なくともアプリケーションは)Encoding以外のクラス名を使わなくて済むように
工夫すればいいと思います。

Updated by Yui NARUSE about 2 years ago

> ライブラリ名は encoding でいいんじゃないですか?

encoding.rb はちょっと時期尚早じゃないかなぁ。

> またクラス名も、実装は Encoding::Detector などわかりやすい名前の下で行いつつ、

その手の Java 的な「わかりやすい名前」が、必ずしも Ruby において「いい名前」ではない、
ってのはこの話の論点の一つですよね。

> APIは(少なくともアプリケーションは)Encoding以外のクラス名を使わなくて済むように
> 工夫すればいいと思います。

その工夫が思いつかなくてたな晒しになっている現状、有効な案だとは思えません。

なお、open 等に detect をつっこめるほど EncDet について知見が集まっているとも考えがたいです。

Updated by Kouhei Sutou about 2 years ago

須藤です。

In <4b0d6903e2a11_8c37d9e412135f0@redmine.ruby-lang.org>
  "[ruby-dev:39781] [Feature #973] EncDet again" on Thu, 26 Nov 2009 02:27:32 +0900,
  Yui NARUSE <redmine@ruby-lang.org> wrote:

>> またクラス名も、実装は Encoding::Detector などわかりやすい名前の下で行いつつ、
> 
> その手の Java 的な「わかりやすい名前」が、必ずしも Ruby において「いい名前」ではない、
> ってのはこの話の論点の一つですよね。

http://doc.okkez.net/static/192/library/_builtin.html
にあるクラス名・モジュール名をざっくり見てみると、クラス名・
モジュール名を省略していない方が多いように見えます。

わかりやすいというのは省略しないということと解釈したのですが、
わかりやすいというのとJava的というのは関係ない気がします。

Updated by Akinori MUSHA about 2 years ago

> > ライブラリ名は encoding でいいんじゃないですか?
> 
> encoding.rb はちょっと時期尚早じゃないかなぁ。

その理由について何らかの材料をいただけないでしょうか。
私の主張はライブラリ名として今使っても問題ないだろうという点なので。

> > またクラス名も、実装は Encoding::Detector などわかりやすい名前の下で行いつつ、
> 
> その手の Java 的な「わかりやすい名前」が、必ずしも Ruby において「いい名前」ではない、
> ってのはこの話の論点の一つですよね。

エンドユーザが使わない部分の名前として挙げたので、そこはどうでもいいと思います。

> > APIは(少なくともアプリケーションは)Encoding以外のクラス名を使わなくて済むように
> > 工夫すればいいと思います。
> 
> その工夫が思いつかなくてたな晒しになっている現状、有効な案だとは思えません。

まずはライブラリ名や実装クラス名の問題を取り除いて、エンドユーザが使うAPIに
フォーカスすればこのissueはシンプルになるのではないでしょうか。

> なお、open 等に detect をつっこめるほど EncDet について知見が集まっているとも考えがたいです。

ご自分が挙げられた IO::detect_open に即座の反対は寄せられていないし、
十分いい名前だと思いますよ。何をdetectするのかという問いは、Encoding自体を
必ずしも文字コードだけに収まらない概念とすれば答えになるでしょう。
(そこの方針を私は把握していないのですが)

[ruby-dev:33628]から早20ヶ月。1.9にはちゃんと番人が居てくれるのだし、
trunkに入れてみて、使ってみては直しを繰り返してこそ問題も見えて来るはず。

私などは新しく書くコードにもNKFを使う有様ですし、Nokogiriなどを見ても、
みんな同じなんだなあと思います。最終版でなくていいから、「いずれこんな感じで
できるようになるよ」というのを見せてほしいとみんな思っていますよ。

Updated by Yui NARUSE about 2 years ago

成瀬です。

Kouhei Sutou wrote:
>>> またクラス名も、実装は Encoding::Detector などわかりやすい名前の下で行いつつ、
>> その手の Java 的な「わかりやすい名前」が、必ずしも Ruby において「いい名前」ではない、
>> ってのはこの話の論点の一つですよね。
> 
> http://doc.okkez.net/static/192/library/_builtin.html
> にあるクラス名・モジュール名をざっくり見てみると、クラス名・
> モジュール名を省略していない方が多いように見えます。

そこにある数々のクラス・モジュールのうち、
自分でその名前を書く物って一部ではないでしょうか。
例えば、BasicObject, Encoding::Converter, File::Constants などが
念頭にあるのだと思いますが、どれも通常書く事はないはずです。

また、略されているのは
ARGF, Bignum, Dir, ENV, Fixnum, GC, Hash, Proc, Regexp
あたりですが、どれも超有名クラスですよね。

> わかりやすいというのは省略しないということと解釈したのですが、
> わかりやすいというのとJava的というのは関係ない気がします。

Java では「わかりやすい名前」(=省略しない名前)を、
「いい名前」であるとしているというイメージがあるので。

Ruby において省略しない事が必ずしもいい事ではなく、
むしろあまり使うべきでないものに対して使われる事が多いのは
前提に置くべきじゃないですかね。

-- 
NARUSE, Yui  <naruse@airemix.jp>

Updated by Yui NARUSE about 2 years ago

成瀬です。

Akinori MUSHA wrote:
>>> ライブラリ名は encoding でいいんじゃないですか?
>> encoding.rb はちょっと時期尚早じゃないかなぁ。
> 
> その理由について何らかの材料をいただけないでしょうか。
> 私の主張はライブラリ名として今使っても問題ないだろうという点なので。

まず前提として、Ruby のエンコーディングの命名は、
IANA Charset の実際上の運用とは異なっています。
つまり、世間では Shift_JIS という名前を Windows-31J として使っているのに対し、
Ruby はその区別を厳格にする事を求め、ルーズにしていると Windows 環境では
例外が上がるように設計されています。

Encoding や、magic comment を読む EncDet はこの枠内で動いています。
ので、ここまでは Encoding に統合可能ではあるのですが、
Encoding をあまり肥大化させると、それ以外の実際上の IANA Charset 的な、
Ruby としては間違った世界のものも扱う必要が出てくるように思います。
その場合の判断は後述の理由から、現時点では避けたいと考えています。

つまり、Encoding の本質について今は判断を避けたいのです。

>>> APIは(少なくともアプリケーションは)Encoding以外のクラス名を使わなくて済むように
>>> 工夫すればいいと思います。
>> その工夫が思いつかなくてたな晒しになっている現状、有効な案だとは思えません。
> 
> まずはライブラリ名や実装クラス名の問題を取り除いて、エンドユーザが使うAPIに
> フォーカスすればこのissueはシンプルになるのではないでしょうか。

これは賛成です。

>> なお、open 等に detect をつっこめるほど EncDet について知見が集まっているとも考えがたいです。
> 
> ご自分が挙げられた IO::detect_open に即座の反対は寄せられていないし、
> 十分いい名前だと思いますよ。何をdetectするのかという問いは、Encoding自体を
> 必ずしも文字コードだけに収まらない概念とすれば答えになるでしょう。
> (そこの方針を私は把握していないのですが)

Encoding は文字コードのみを扱うべきだと、わたしは現時点で思っています。

> 私などは新しく書くコードにもNKFを使う有様ですし、Nokogiriなどを見ても、
> みんな同じなんだなあと思います。最終版でなくていいから、「いずれこんな感じで
> できるようになるよ」というのを見せてほしいとみんな思っていますよ。

Nokogiri は前述の IANA Charset ベースの話なので、EncDet より話は悲惨です。
つまり、charset=Shift_JIS まわりで地雷を踏むことでしょう。

こっちがまともになるのはもうしばらくかかると思われます。
この辺について、IANA 側で動く気配があるので、そちらの動きが見えるまでは
Ruby 側で対処するつもりはありません。

-- 
NARUSE, Yui  <naruse@airemix.jp>

Updated by Kouhei Sutou about 2 years ago

須藤です。

# なんか、本質的な話じゃない流れな気がしますが。。。

In <4B0D7B56.9000002@airemix.jp>
  "[ruby-dev:39784] Re: [Feature #973] EncDet again" on Thu, 26 Nov 2009 03:45:54 +0900,
  "NARUSE, Yui" <naruse@airemix.jp> wrote:

>> http://doc.okkez.net/static/192/library/_builtin.html
>> にあるクラス名・モジュール名をざっくり見てみると、クラス名・
>> モジュール名を省略していない方が多いように見えます。
> 
> そこにある数々のクラス・モジュールのうち、
> 自分でその名前を書く物って一部ではないでしょうか。
> 例えば、BasicObject, Encoding::Converter, File::Constants などが
> 念頭にあるのだと思いますが、どれも通常書く事はないはずです。
> 
> また、略されているのは
> ARGF, Bignum, Dir, ENV, Fixnum, GC, Hash, Proc, Regexp
> あたりですが、どれも超有名クラスですよね。

Bignum, Fixnum, GC(!), Hash, (Proc,) Regexpは通常書くことは
ないと思います。というのはよいとして、略されているものでも、
そういう風に略されることが多いものな気がします。(ARGFは違い
ますが。)


通常書くことがあり、略されていない超有名クラスもFile,
Thread(?), Time(?), LoadErrorなどがあったりします。

略すにしてもそれがなんなのかが想像できるものがよいなぁと思い
ます。私にはDetは難しいです。

> Ruby において省略しない事が必ずしもいい事ではなく、
> むしろあまり使うべきでないものに対して使われる事が多いのは
> 前提に置くべきじゃないですかね。

そういうわけじゃないと思っていました。
まずは、省略せずにしておいて、よく使うから短くしたいなぁ、じゃ
あ、短くするか、という流れかと思っていました。

エンコーディング検出処理は、どのくらいよく使われる処理。。。
なんですかねぇ。どうなのかしら。

Updated by Yui NARUSE about 2 years ago

Kouhei Sutou wrote:
> 略すにしてもそれがなんなのかが想像できるものがよいなぁと思い
> ます。私にはDetは難しいです。

初期の名称案に EncDetect はありますね。
これでもダメですか。

> エンコーディング検出処理は、どのくらいよく使われる処理。。。
> なんですかねぇ。どうなのかしら。

[ruby-dev:37679] にて、標準ライブラリ内だけで三例報告されています。
> さて、現在私が知る限りRDocとERBとIRBがそれぞれ独自にマジックコメントを解
> 釈してファイルを開く機能を実装しています。この重複具合は何らかの共通化の
> 必要性を示しているのではないかと思います。

-- 
NARUSE, Yui  <naruse@airemix.jp>

Updated by Kouhei Sutou about 2 years ago

In <4B0FF194.2030809@airemix.jp>
  "[ruby-dev:39804] Re: [Feature #973] EncDet again" on Sat, 28 Nov 2009 00:34:59 +0900,
  "NARUSE, Yui" <naruse@airemix.jp> wrote:

> Kouhei Sutou wrote:
>> 略すにしてもそれがなんなのかが想像できるものがよいなぁと思い
>> ます。私にはDetは難しいです。
> 
> 初期の名称案に EncDetect はありますね。
> これでもダメですか。
> 
>> エンコーディング検出処理は、どのくらいよく使われる処理。。。
>> なんですかねぇ。どうなのかしら。
> 
> [ruby-dev:37679] にて、標準ライブラリ内だけで三例報告されています。
>> さて、現在私が知る限りRDocとERBとIRBがそれぞれ独自にマジックコメントを解
>> 釈してファイルを開く機能を実装しています。この重複具合は何らかの共通化の
>> 必要性を示しているのではないかと思います。

私にとって3例は多くないので、Encと略すほどではないと感じます。

Updated by Yuki Sonoda about 2 years ago

2009/11/28 Kouhei Sutou <kou@cozmixng.org>:
> 私にとって3例は多くないので、Encと略すほどではないと感じます。

Encodingクラスがあるのに、EncXXXを定義するのはとても嫌です。
成瀬さんの懸念も分かりますので encoding.rbは避けるとしても、やはりEncoding::XXXであって欲しいです。

ところで、先日IRCで挙がった話題ですが、EncDetは何を提供するのでしょうか。
当初提案されたEncDetの機能はマジックコメントとBOMの解釈でした。このうち、BOMは既にFile.openに実装されています。
ですから、今想定されるのはファイルの最初の一行からマジックコメントっぽいバイト列を探してそれに基づいて開く機能のみです。
この機能が複数箇所で実装されているのでこれは、何らかの良いデフォルト実装を提供しなければならないというのが私の主張でした。
この機能だけであるとすると、実は当面File.openの機能拡張のみで済んでしまうのではないでしょうか。たとえば、

File.open(path, "r:magic-comment")
ないし
File.open(path, "r:auto")
のようにして。

-- 
Yuki Sonoda (Yugui)
yugui@yugui.jp
http://yugui.jp

Updated by Yusuke Endoh almost 2 years ago

遠藤です。

EncDet の件、どうしますか?
Yugui さんが以下のような提案もしています。

> 実は当面File.openの機能拡張のみで済んでしまうのではないでしょうか。たとえば、
>
> File.open(path, "r:magic-comment")
> ないし
> File.open(path, "r:auto")
> のようにして。


さて、

  1) EncDet 方式で決定する (名前はまつもとさんの好みで決める)
  2) File.open 方式で決定する
  3) 1.9.2 を見送ってじっくり議論する

のどれにしましょうか。


2 回も名前の議論で発散したようなので、議論を再開するより、まつもと
さんの好みで決めてしまうのがよいのではないかと思います。

3 月末までに決定できないと自動的に #3 になります。


#2 の選択肢は実現可能性が検証されていない気がするので、「パッチを
書いてみたら実は難しいことがわかった → 1.9.2 見送り」という危険が
あるかもしれません。

-- 
Yusuke ENDOH <mame@tsg.ne.jp>

Updated by Nobuyoshi Nakada almost 2 years ago

なかだです。

At Wed, 17 Mar 2010 22:46:43 +0900,
Yusuke Endoh wrote in [ruby-dev:40687]:
>   2) File.open 方式で決定する

> #2 の選択肢は実現可能性が検証されていない気がするので、「パッチを
> 書いてみたら実は難しいことがわかった → 1.9.2 見送り」という危険が
> あるかもしれません。

別に難しくはありません。

$ ./ruby -Eus-ascii -e 'ARGV.each{|file|open(file, "r:magic-comment"){|f|p [f.path, f.external_encoding]}}' version.h lib/rexml/rexml.rb lib/rubygems/package.rb 
["version.h", #<Encoding:US-ASCII>]
["lib/rexml/rexml.rb", #<Encoding:UTF-8>]
["lib/rubygems/package.rb", #<Encoding:ISO-8859-1>]


diff --git c/include/ruby/io.h i/include/ruby/io.h
index e05a0f5..f067831 100644
--- c/include/ruby/io.h
+++ i/include/ruby/io.h
@@ -96,4 +96,5 @@ typedef struct rb_io_t {
 /* #define FMODE_PREP               0x00010000 */
 #define FMODE_SETENC_BY_BOM         0x00100000
+#define FMODE_SETENC_BY_MAGIC_COMMENT 0x00200000

 #define GetOpenFile(obj,fp) rb_io_check_closed((fp) = RFILE(rb_io_taint_check(obj))->fptr)
diff --git c/io.c i/io.c
index 60afd6c..60761f0 100644
--- c/io.c
+++ i/io.c
@@ -4125,4 +4125,6 @@ rb_io_ext_int_to_encs(rb_encoding *ext, rb_encoding *intern, rb_encoding **enc,
 }

+#define is_magic_comment(str) (STRCASECMP(str, "magic-comment") == 0)
+
 static void
 parse_mode_enc(const char *estr, rb_encoding **enc_p, rb_encoding **enc2_p)
@@ -4166,5 +4168,5 @@ parse_mode_enc(const char *estr, rb_encoding **enc_p, rb_encoding **enc2_p)
 	ext_enc = rb_enc_from_index(idx);
     else {
-	if (idx != -2)
+	if (idx != -2 && !is_magic_comment(estr))
 	    rb_warn("Unsupported encoding %s ignored", estr);
 	ext_enc = NULL;
@@ -4337,6 +4339,11 @@ rb_io_extract_modeenc(VALUE *vmode_p, VALUE *vperm_p, VALUE opthash,
             has_enc = 1;
             parse_mode_enc(p+1, &enc, &enc2);
-	    if (io_encname_bom_p(p+1, 0))
+	    if (io_encname_bom_p(p+1, 0)) {
 		fmode |= FMODE_SETENC_BY_BOM;
+		p += 4;
+	    }
+	    if (is_magic_comment(p+1)) {
+		fmode |= FMODE_SETENC_BY_BOM | FMODE_SETENC_BY_MAGIC_COMMENT;
+	    }
         }
 	else {
@@ -4605,10 +4612,44 @@ io_strip_bom(VALUE io)
 }

-static void
-io_set_encoding_by_bom(VALUE io)
+int rb_magic_comment_encoding(const char *str, long len);
+
+static int
+io_parse_encoding_comment(VALUE io)
 {
-    int idx = io_strip_bom(io);
+    VALUE line = rb_io_gets(io);
+    char *s;
+    long n;
+    if (NIL_P(line)) return 0;
+    s = RSTRING_PTR(line);
+    n = RSTRING_LEN(line);
+    if (n >= 2 && s[0] == '#' && s[1] == '!') {
+	VALUE shbang = line;
+	line = rb_io_gets(io);
+	if (NIL_P(line)) {
+	    rb_io_ungetbyte(io, shbang);
+	    return 0;
+	}
+	rb_io_ungetbyte(io, line);
+	s = RSTRING_PTR(line);
+	n = RSTRING_LEN(line);
+	line = shbang;
+    }
+    rb_io_ungetbyte(io, line);
+    while (n > 0 && (*s == ' ' || *s == '\t')) {
+	s++;
+	n--;
+    }
+    if (n <= 0 || *s != '#') return 0;
+    return rb_magic_comment_encoding(s, n);
+}

-    if (idx) {
+static void
+io_guess_encoding(VALUE io, int fmode)
+{
+    int idx;
+    if (((fmode & FMODE_SETENC_BY_BOM) &&
+	 (idx = io_strip_bom(io)) != 0) ||
+	((fmode & FMODE_SETENC_BY_MAGIC_COMMENT) &&
+	 (idx = io_parse_encoding_comment(io)) != 0)) {
 	rb_io_t *fptr;
 	GetOpenFile(io, fptr);
@@ -4638,5 +4679,5 @@ rb_file_open_generic(VALUE io, VALUE filename, int oflags, int fmode, convconfig
     fptr->fd = rb_sysopen(fptr->pathv, oflags, perm);
     io_check_tty(fptr);
-    if (fmode & FMODE_SETENC_BY_BOM) io_set_encoding_by_bom(io);
+    io_guess_encoding(io, fmode);

     return io;
@@ -6396,5 +6437,5 @@ rb_io_initialize(int argc, VALUE *argv, VALUE io)
 	fp->stdio_file = stderr;

-    if (fmode & FMODE_SETENC_BY_BOM) io_set_encoding_by_bom(io);
+    io_guess_encoding(io, fmode);
     return io;
 }
diff --git c/parse.y i/parse.y
index 340a825..a42c8f6 100644
--- c/parse.y
+++ i/parse.y
@@ -6248,13 +6248,15 @@ magic_comment_marker(const char *str, long len)
 }

+typedef int rb_magic_comment_func(const char *name, long nlen, const char *value, long vlen, void *arg);
+
 static int
-parser_magic_comment(struct parser_params *parser, const char *str, long len)
+parse_magic_comment(const char *str, long len, rb_magic_comment_func *func, void *arg)
 {
-    VALUE name = 0, val = 0;
+    VALUE name = 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))))
+	: ((_s) = rb_str_new((_p), (_n))))

     if (len <= 7) return FALSE;
@@ -6266,7 +6268,4 @@ parser_magic_comment(struct parser_params *parser, const char *str, long len)
     /* %r"([^\\s\'\":;]+)\\s*:\\s*(\"(?:\\\\.|[^\"])*\"|[^\"\\s;]+)[\\s;]*" */
     while (len > 0) {
-#ifndef RIPPER
-	const struct magic_comment *p = magic_comments;
-#endif
 	char *s;
 	int i;
@@ -6321,24 +6320,68 @@ parser_magic_comment(struct parser_params *parser, const char *str, long len)
 	    if (s[i] == '-') s[i] = '_';
 	}
+	if ((*func)(s, n, vbeg, vend - vbeg, arg)) break;
+    }
+
+    return TRUE;
+}
+
+static int
+magic_comment_i(const char *name, long nlen, const char *value, long vlen, void *arg)
+{
+    struct parser_params *parser = arg;
 #ifndef RIPPER
-	do {
-	    if (STRNCASECMP(p->name, s, n) == 0) {
-		n = vend - vbeg;
-		if (p->length) {
-		    n = (*p->length)(parser, vbeg, n);
-		}
-		str_copy(val, vbeg, n);
-		(*p->func)(parser, s, RSTRING_PTR(val));
-		break;
+    const struct magic_comment *p = magic_comments;
+    do {
+	if (STRNCASECMP(p->name, name, nlen) == 0) {
+	    char *val;
+	    if (p->length) {
+		vlen = (*p->length)(parser, value, vlen);
 	    }
-	} while (++p < magic_comments + numberof(magic_comments));
+	    val = ALLOCA_N(char, vlen + 1);
+	    memcpy(val, value, vlen);
+	    val[vlen] = '\0';
+	    (*p->func)(parser, name, val);
+	    break;
+	}
+    } while (++p < magic_comments + numberof(magic_comments));
 #else
-	dispatch2(magic_comment, name, val);
+    dispatch2(magic_comment, name, val);
 #endif
-    }

+    return FALSE;
+}
+
+static int
+parser_magic_comment(struct parser_params *parser, const char *str, long len)
+{
+    return parse_magic_comment(str, len, magic_comment_i, (void *)parser);
+}
+
+static int
+find_magic_comment_encoding(const char *name, long nlen, const char *value, long vlen, void *arg)
+{
+    char *val;
+    switch (nlen) {
+      case 8:
+	if (STRNCASECMP("en", name, 2) != 0) return FALSE;
+	name += 2;
+      case 6:
+	if (STRNCASECMP("coding", name, 6) != 0) return FALSE;
+    }
+    vlen = parser_encode_length(0, value, vlen);
+    memcpy(val = ALLOCA_N(char, vlen + 1), value, vlen);
+    val[vlen] = '\0';
+    *(int *)arg = rb_enc_find_index(val);
     return TRUE;
 }

+int
+rb_magic_comment_encoding(const char *str, long len)
+{
+    int idx = 0;
+    if (!parse_magic_comment(str, len, find_magic_comment_encoding, &idx)) return 0;
+    return idx;
+}
+
 static void
 set_file_encoding(struct parser_params *parser, const char *str, const char *send)


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦

Updated by Yusuke Endoh almost 2 years ago

遠藤です。

2010年3月18日7:05 Nobuyoshi Nakada <nobu@ruby-lang.org>:
> At Wed, 17 Mar 2010 22:46:43 +0900,
> Yusuke Endoh wrote in [ruby-dev:40687]:
>> ? 2) File.open 方式で決定する
>
>> #2 の選択肢は実現可能性が検証されていない気がするので、「パッチを
>> 書いてみたら実は難しいことがわかった → 1.9.2 見送り」という危険が
>> あるかもしれません。
>
> 別に難しくはありません。


なるほど。もうパッチがあるということなら #2 でもいいかもですね。

正直に言うと、本気で実現可能性を疑っていたわけではなく、選択肢を
増やしても決定の可能性が下がるだけじゃないかと思ってのことでした。

再度いいますが、3 月末までに合意できないと自動的に流れますので、
導入を狙っている人たちはがんばってください。

-- 
Yusuke ENDOH <mame@tsg.ne.jp>

Updated by Yuki Sonoda almost 2 years ago

2010/3/20 Yusuke ENDOH <mame@tsg.ne.jp>:
> 再度いいますが、3 月末までに合意できないと自動的に流れますので、
> 導入を狙っている人たちはがんばってください。

議論の過程でもうちょっと実例が集まることを期待したんですが、結局3例のままですよね。
ならば流してもうちょっと慎重に検討しても良いのではないでしょうか。まずはredmineにfeatureとして登録されたことで将来的に検討される可能性が残ったということで私は満足です。

-- 
Yuki Sonoda (Yugui)
yugui@yugui.jp
http://yugui.jp

Updated by Yusuke Endoh almost 2 years ago

  • Target version changed from 1.9.2 to 2.0.0
遠藤です。

2010年3月20日16:00 Yugui <yugui@yugui.jp>:
> 2010/3/20 Yusuke ENDOH <mame@tsg.ne.jp>:
>> 再度いいますが、3 月末までに合意できないと自動的に流れますので、
>> 導入を狙っている人たちはがんばってください。
>
> 議論の過程でもうちょっと実例が集まることを期待したんですが、結局3例のままですよね。
> ならば流してもうちょっと慎重に検討しても良いのではないでしょうか。まずはredmineにfeatureとして登録されたことで将来的に検討される可能性が残ったということで私は満足です。


ということなので、target を 1.9.x にさせていただきます。


個人的には標準ライブラリ中に 3 例 (RDoc, ERB, IRB) もあれば十分な気が
しますし、[ruby-dev:39779] の成瀬さんによると、

> しかし、現状rdoc絡みやerb周りなど、EncDetを再発明しようとした挙句に失敗してしまった例が散見されており、
> そろそろこのライブラリは標準添付しないと悪しき遺産を残しかねないと憂慮しています。
>
> 言い換えると、このライブラリの用途は様々な場面で必要である一方、実装が意外と難しいので、
> 適切なライブラリを標準添付で提供しなければならないと思っています。
> たとえ名前で合意がつかなかったとしても、yuguiさん判断で名前を決定し、添付するべきであろうと。

とのことなので、入れた方がいいんじゃないかなと思っていました。

-- 
Yusuke ENDOH <mame@tsg.ne.jp>

Updated by Yuki Sonoda almost 2 years ago

2010/3/22 Yusuke Endoh <redmine@ruby-lang.org>:
>> 言い換えると、このライブラリの用途は様々な場面で必要である一方、実装が意外と難しいので、
>> 適切なライブラリを標準添付で提供しなければならないと思っています。
>> たとえ名前で合意がつかなかったとしても、yuguiさん判断で名前を決定し、添付するべきであろうと。
>
> とのことなので、入れた方がいいんじゃないかなと思っていました。

本当にこの(2)のインターフェースで良いのかもう少し事例がないと自信を持てないんですね。
悪しき実装がはびこるのも頭が痛いですが、まだ3例以外にはびこってる例が見あたらないので待っても良いのではないでしょうか。

-- 
Yuki Sonoda (Yugui)
yugui@yugui.jp
http://yugui.jp

Updated by Shyouhei Urabe over 1 year ago

  • Status changed from Open to Assigned

Updated by Motohiro KOSAKI 2 days ago

>> 実は当面File.openの機能拡張のみで済んでしまうのではないでしょうか。たとえば、 >> >> File.open(path, "r:magic-comment") >> ないし >> File.open(path, "r:auto") >> のようにして。 すでにパッチが投稿されているようですが、これでダメな理由がよく分かりません。実装があるものに対してrejectはないかなと思っているのでまつもとさんがコメントしないと永久に棚晒しじゃないですかねえ

Also available in: Atom PDF