✅ Build sakura 1.0.2406 completed (commit 3d3a6dfcbe by @berryzplus)

説明に試食版と書かれてますが merge 先が master ブランチになっています。

気になる点としてはオプショナルな実装になっているかですね。

• ICU4Cが無くてもビルド出来るかどうか
• ICU4Cが無くても生成物が動作するか
• ICU4Cが存在する環境でも使わないように設定する事で従来通りの動作に出来るか
  • dllファイルを配置するかどうかで制御みたいですが。。

あとメリットの説明にはICU4Cを使うことで判別精度が良くなるとかは書かれていないので、使うことが目的に見えてしまいます、手段が目的になるのはよくあることですが。PRの影響範囲には利点が少し書かれてますね。

説明に試食版と書かれてますが merge 先が master ブランチになっています。

意図通りです。現状では、マージしても一般の人が使うのは難しい実装になっています。
ビルドにも実行にもICU4Cは不要なので、masterにマージしても問題ないと考えています。
なお、CIのMinGWビルドが失敗しているのは #1108 の影響です。

気になる点としてはオプショナルな実装になっているかですね。
•ICU4Cが無くてもビルド出来るかどうか
•ICU4Cが無くても生成物が動作するか
•ICU4Cが存在する環境でも使わないように設定する事で従来通りの動作に出来るか
◦dllファイルを配置するかどうかで制御みたいですが。。

設定項目は既に飽和していると思うので、設定により無効化する機能は投入していません。
隠し設定（＝GUIなし）で無効化する機能ならば入れても良いかも知れません。
いずれにしろ、現状は「有効にする」のが非常に難しいと思うので本体に入っても「お試し」に過ぎない認識です。

あとメリットの説明にはICU4Cを使うことで判別精度が良くなるとかは書かれていないので、使うことが目的に見えてしまいます、手段が目的になるのはよくあることですが。PRの影響範囲には利点が少し書かれてますね。

バイナリファイルを読み込ませたときの挙動をどうすべきかで迷いがあったのでメリットには書きませんでした。先頭数文字がASCIIでファイルの途中にNULバイトが含まれるようなファイル(BMPやEXEなど)を読み込ませた場合とか、現状だと LATIN1 になっちゃうのを対策したほうがいいんじゃないか？みたいな迷いが。

テキストファイルの判別精度は上がると考えられます。
ただし、判別したキャラクタセットをすべて拾い切れるわけではないので、変換部をもう少し詰める必要があると思います。

現状の実装ではサクラエディタの内部コードに変換できなかった場合は、従来通りの判別ロジックが走るようにしています。

Oh....

g++ -finput-charset=utf-8 -fexec-charset=cp932 -I. -DWIN32 -D_WIN32_WINNT=_WIN32_WINNT_WIN7 -D_UNICODE -DUNICODE -D_DEBUG -g -O0 -std=c++17  -o charset/CEuc.o -c charset/CEuc.cpp
   43 |  typedef UCharsetDetector*  (_cdecl *ucsdet_open_t)(UErrorCode *status);
      |                             ~      ^~
      |                                    )

ローカルで再現しそうなビルドエラーが出ていたことに気付くなど...orz

✅ Build sakura 1.0.2412 completed (commit 1d11416df5 by @berryzplus)

/azp run

/azp run

/azp run

文字コード判定で libguess というのもあるみたいなので、巨大な ICU4C を使うより出来るだけ小規模なライブラリを取り込む方が良いように思えます。どちらも使った事無いので無責任な発言ですけど。。

「嫌」ですかね？ｗ

一応世間的には、ICU4Cを使うやり方がもっともド真ん中正攻法なやり方なはずです。

PR出しといてアレですが、個人的な見解としては「他に選択肢がなくて仕方なく採用」というのがICU4Cへの評価です。可能ならば使いたくはない、でもUNICODE規格への完全対応を視野にいれるなら避けては通れない、それがICU4Cだと思っています。GitHubのプロジェクトをウォッチしてますけど、まともなDLLを作れるようになるまでにあと10年はかかりそうな気配を感じています。

で、このPRをどうするか？

• このPRはICU4Cの文字コード判別機構をサクラエディタで使えるようにします。
• このPRによる変更で、ICU4Cのヘッダ・ライブラリへの依存は発生しません。
• このPRによる機能追加は「とりあえず使う」には十分ですが、改善点がたくさんあります。

「とりあえず入れちゃっても害はない」と思うので入れちゃいたい感じですが、rejectでもよいです。rejectなら前提として作った CCodeMediator のリファクタリングを切り離して別出ししたい感じですが、他のライブラリ参照系機能のベースとして参考になるソースだと思うので、PR自体はしばらく放置しとく感じにすると思います。

• icudt66.dll
• icuin66.dll
• icuuc66.dll

ですが、合計して 3.67 MB ぐらいでした。

文字コード判別だけならもっと規模が小さいライブラリで実現した方が良いと思いますが、追加したいならOKだと思います。

ICU4Cによる文字コード検出機能を追加、以外の変更も含まれているのでちょっと比較しなきゃいけない差分が多いのが面倒に感じますね。。

すんません、頂いたコメントへのレスは明日になりそうです。

✅ Build sakura 1.0.2416 completed (commit 1439eee1a8 by @berryzplus)

•icudt66.dll
•icuin66.dll
•icuuc66.dll

ですが、合計して 3.67 MB ぐらいでした。

これなんですけど、うちのローカルビルドで生成された数字を参考で載せときます。

Release | Win32 = 30.3 MB (31,835,648 bytes)
Release | x64 = 31.0 MB (32,508,416 bytes)

•icudt66.dll
•icuin66.dll
•icuuc66.dll
ですが、合計して 3.67 MB ぐらいでした。

これなんですけど、うちのローカルビルドで生成された数字を参考で載せときます。

Release | Win32 = 30.3 MB (31,835,648 bytes)
Release | x64 = 31.0 MB (32,508,416 bytes)

およ、不思議ですね…。

ちなみに自分がビルドしたのは 873e2db78070d3015a54f9d9aa81e97095cd9dba の icu4c/source/allinone.sln の Win32 Release です。生成物は icu4c/bin に作られました。

およ、不思議ですね…。

ちなみに自分がビルドしたのは 873e2db78070d3015a54f9d9aa81e97095cd9dba の icu4c/source/allinone.sln の Win32 Release です。生成物は icu4c/bin に作られました。

コミットハッシュ ⇒ 3b99d07581fb38a68b9a786abab0785b4e3d3749
使ったソリューションと出力先は同じっぽいです。
vs2017だから？・・・いや、10倍になるのは流石におかしいか。
あ、 python3 入れてないと icudt66.dll がデータを含まないダミーになるのでそれが原因かもです。

多分問題無いと思います。

PRの件名や説明に書かれていないリファクタリングの変更が事前に行われてるのでちょっと確認しづらいですが、まぁ問題があれば後で直せば良いと思います。

あざっす。

やったこと全部書かないとレビューしてもらえない感じだとつらくなってきたので、敢えて説明を端折ってみてる部分もありました。何かあったら「なる早」で対応するのは、どうレビューしても変わらないはずなので、あまり負荷にならない範囲で端折っていきたいと思っています。

やったこと全部書かないとレビューしてもらえない感じだとつらくなってきたので、敢えて説明を端折ってみてる部分もありました。何かあったら「なる早」で対応するのは、どうレビューしても変わらないはずなので、あまり負荷にならない範囲で端折っていきたいと思っています。

リファクタとそれ以外でPRを分けた方がレビューする上では良かったかもですね。

4年くらい前、ICU 54の頃に試したときには、Data Library Customizerという機能があって、フルビルドすると数十MBのDLLが、Base Dataのみでビルドすると0.5MB程度に抑えることができました。（最新版でCustomizerが提供されているのか、されているとすれば文字コード検出のためにはどこまでデータを削れるのか…）
もし、データを削って数MB程度に抑えることができるのであれば、インストーラーに同梱するのも選択肢に入るでしょう。

あと、ICU 66はまだリリースされていないので、現時点で採用するならICU 65.1ではないでしょうか。
https://github.com/unicode-org/icu/releases/tag/release-65-1 (バイナリもちゃんと提供されているようですし)

文字コード検出の為だけにインストーラーのサイズが数MBも増えるのは富豪的な気がしますが、今の人の感覚では数MBなんてブロードリンク元社員の遵法精神より軽いかもしれないですね。

レビューありがとうございます。
何か問題ありましたら別PRで対処していきたいと思います。

具体的な課題について個別対処できそうな雰囲気になってきたので、一旦マージしてしまいます。

文字コード検出の為だけにインストーラーのサイズが数MBも増えるのは富豪的な気がしますが、今の人の感覚では数MBなんてブロードリンク元社員の遵法精神より軽いかもしれないですね。

よくよく考えてみると Visual Studio 2017 のインストーラーがダウンロードするパッケージのサイズを合計すると テラバイト単位 なんですよね。そこを考えると、インターネット配布で数十MBなんてあり得ない って発想が適切なのかどうかは怪しい気がしてきます。

icu4cのバージョンアップ頻度は結構高いので、何にもしなくてもver66(未リリース)はそのうち公開されると思っています。たぶん「単一バージョンのみに対応」って方式はキッツイ感じだと思います。そこを発展させてデュアルバージョン対応にすると、他のexmoduleと乖離していくので一旦そのままにしてしまったんですが。

え、テラバイトって 1024GB ですよね？
最近になって1TBの容量のSSDの値段が1万円くらいになりましたが…。

まぁでも最近の開発環境って数10GB消費するのも珍しくないし、そのうち10TBじゃドライブの容量が足りないとかいう時代が来るんでしょうか…。

ギガでしたね。

最大で40GB、ブルーレイディスク１枚にぎりぎり入るサイズだったような。