ICU4Cによる文字コード検出機能を追加する #1104

berryzplus · 2019-11-30T12:04:59Z

PR の目的

ICU4Cによる文字コード検出機能を追加

カテゴリ

機能追加

PR の背景

ICU というライブラリがあります。
http://site.icu-project.org/home

IBMが作ったライブラリで、ほぼすべての文明国のローカル言語を表現できる文字コードセット群をサポートしています。一応、何年も前に IBM の手を離れていて、現在は Unicode コンソーシアムが仕切りをやっているので、最新のUNICODE規格にも対応しているはずです。

c++規格がwchar_tをUNICODE規格に準拠させる対応を積極的に行わない理由。
UNICODE規格は、１プログラム言語の仕様として取り込むには複雑すぎる、らしいです。
その「複雑すぎる仕様」をコードの形に落とし込んであるのが ICU4C です。

試食版として、とりあえず動くように組み込んでみました。
最新版DLL(icuin66.dll, icudt66.dll, icuuc66.dll) はまだダウンロードできないっぽいので、動作させるには https://github.com/unicode-org/icu からソースを落としてビルドする必要があります。

PR のメリット

文字コード判別に ICU4C の DLL を使えるようになります。

PR のデメリット (トレードオフとかあれば)

ICU4C はUNIX由来のプロジェクトなので、Windows向けポートが「雑」です。
ICU4C の DLL にはインターフェース関数がないので、対応バージョンは固定にせざるを得ませんでした。PRの変更では、現時点の最新版(＝66)のみに対応するようになっています。
ICU4C の DLL はインターネットで配布するには巨大すぎる (約20MB) ので、インストーラーに同梱する予定はありません。これにより次の問題が発生します。
この PR による変更を確認するために必要な ICU4C の DLL は icuin66.dll, icudt66.dll, icuuc66.dll の 3つですが、これらを github から落としてきて使える状態にする作業は「一般向け」としては難易度が高すぎるように思います。

メリットは ICU4Cが使えるようになること なんですが、デメリットは ICU4Cを使わざるを得ないこと だったりします。

PR の影響範囲

この PR の恩恵を得るには、ICU4C の DLL が必要です。
- ICU4C の DLL を入手しない場合、アプリ（＝サクラエディタ）の機能に影響はありません。
- 入手した DLL は sakura.exe と同じフォルダに配置する必要があります。。
(ICU4C の DLL を用意した場合のみ) 文字コード判別に ICU4C が使われるようになります。
- 主に通常用途(テキストの編集など) では ICU4C のコード判別のほうが有利です。
- 変態用途(バイナリファイルの編集など) ではサクラエディタのコード判別でないと使い物にならないと考えられます。

参考資料

Mediatorパターン

CCodeMediator.hからCESI.h依存を追い出して仲介者としての役割を果たせるようにする。

AppVeyorBot · 2019-11-30T12:21:11Z

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

beru · 2019-11-30T15:45:41Z

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

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

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

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

berryzplus · 2019-11-30T16:12:06Z

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

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

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

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

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

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

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

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

berryzplus · 2019-11-30T16:59:27Z

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

誤) _cdecl 正) __cdecl 呼出規約を表すキーワード__cdeclはアンダースコア2つが正しいです。

AppVeyorBot · 2019-11-30T17:27:11Z

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

berryzplus · 2019-11-30T17:39:25Z

/azp run

azure-pipelines · 2019-11-30T17:39:51Z

Azure Pipelines successfully started running 1 pipeline(s).

berryzplus · 2019-12-01T04:18:45Z

/azp run

azure-pipelines · 2019-12-01T04:19:11Z

Azure Pipelines successfully started running 1 pipeline(s).

berryzplus · 2019-12-01T06:52:31Z

/azp run

azure-pipelines · 2019-12-01T06:53:00Z

Azure Pipelines successfully started running 1 pipeline(s).

beru · 2019-12-01T09:14:51Z

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

berryzplus · 2019-12-01T13:52:27Z

「嫌」ですかね？ｗ

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

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

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

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

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

beru · 2019-12-01T15:14:58Z

icudt66.dll
icuin66.dll
icuuc66.dll

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

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

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

sakura_core/charset/CCodeMediator.cpp

sakura_core/charset/CESI.h

berryzplus · 2019-12-04T15:47:45Z

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

BOMコードが間違っていたのをこっそり修正。

AppVeyorBot · 2019-12-07T06:16:05Z

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

berryzplus · 2019-12-07T06:27:42Z

•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)

beru · 2019-12-07T09:41:59Z

•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 に作られました。

beru

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

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

berryzplus · 2019-12-07T10:26:37Z

およ、不思議ですね…。

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

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

berryzplus · 2019-12-07T10:32:31Z

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

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

あざっす。

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

beru · 2019-12-08T04:11:17Z

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

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

k-takata · 2019-12-08T05:15:10Z

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 (バイナリもちゃんと提供されているようですし)

beru · 2019-12-08T05:27:00Z

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

berryzplus · 2019-12-08T06:33:01Z

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

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

berryzplus · 2019-12-08T06:44:14Z

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

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

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

beru · 2019-12-08T06:54:21Z

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

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

berryzplus · 2019-12-08T08:08:36Z

ギガでしたね。

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

…4c_charsetdetector ICU4Cによる文字コード検出機能を追加する

berryzplus added 2 commits November 30, 2019 19:10

CCodeMediatorリファクタリング

00410f2

CCodeMediator.hからCESI.h依存を追い出して仲介者としての役割を果たせるようにする。

ICU4Cによる文字コード検出機能を追加

fc94768

スペルミス訂正

82c8252

誤) _cdecl 正) __cdecl 呼出規約を表すキーワード__cdeclはアンダースコア2つが正しいです。

beru reviewed Dec 4, 2019

View reviewed changes

sakura_core/charset/CCodeMediator.cpp Show resolved Hide resolved

beru reviewed Dec 4, 2019

View reviewed changes

sakura_core/charset/CESI.h Show resolved Hide resolved

berryzplus changed the title ~~ICU4Cによる文字コード検出機能を追加する~~ [WIP] ICU4Cによる文字コード検出機能を追加する Dec 7, 2019

BOMコードに関する説明を拡充

da6037b

BOMコードが間違っていたのをこっそり修正。

berryzplus changed the title ~~[WIP] ICU4Cによる文字コード検出機能を追加する~~ ICU4Cによる文字コード検出機能を追加する Dec 7, 2019

beru approved these changes Dec 7, 2019

View reviewed changes

berryzplus merged commit 2c6612c into sakura-editor:master Dec 8, 2019

berryzplus deleted the feature/add_icu4c_charsetdetector branch December 8, 2019 06:33

m-tmatma added this to the v2.4.0 milestone Dec 29, 2019

KENCHjp added the enhancement ■機能追加 label Jan 7, 2020

berryzplus mentioned this pull request Jan 26, 2020

使用しないメソッド引数のマーキングにmaybe_unused属性を使う #1174

Merged

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 16, 2020

Merge pull request sakura-editor#1104 from berryzplus/feature/add_icu…

9ac1aa5

…4c_charsetdetector ICU4Cによる文字コード検出機能を追加する

beru mentioned this pull request Sep 20, 2021

uchardet.dll が存在したらそれを使って文字エンコーディングの検出が行われるように処理追加 #1726

Merged

This was referenced Jan 29, 2022

ICU4CをWindows10同梱のものに差し替える検討 #1783

Closed

文字コード自動判別にwin10組み込みのICU4Cを使う #1784

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ICU4Cによる文字コード検出機能を追加する #1104

ICU4Cによる文字コード検出機能を追加する #1104

berryzplus commented Nov 30, 2019

AppVeyorBot commented Nov 30, 2019

beru commented Nov 30, 2019

berryzplus commented Nov 30, 2019

berryzplus commented Nov 30, 2019

AppVeyorBot commented Nov 30, 2019

berryzplus commented Nov 30, 2019

azure-pipelines bot commented Nov 30, 2019

berryzplus commented Dec 1, 2019

azure-pipelines bot commented Dec 1, 2019

berryzplus commented Dec 1, 2019

azure-pipelines bot commented Dec 1, 2019

beru commented Dec 1, 2019

berryzplus commented Dec 1, 2019

beru commented Dec 1, 2019

berryzplus commented Dec 4, 2019

AppVeyorBot commented Dec 7, 2019

berryzplus commented Dec 7, 2019

beru commented Dec 7, 2019

beru left a comment

berryzplus commented Dec 7, 2019

berryzplus commented Dec 7, 2019

beru commented Dec 8, 2019

k-takata commented Dec 8, 2019

beru commented Dec 8, 2019

berryzplus commented Dec 8, 2019

berryzplus commented Dec 8, 2019

beru commented Dec 8, 2019

berryzplus commented Dec 8, 2019

ICU4Cによる文字コード検出機能を追加する #1104

ICU4Cによる文字コード検出機能を追加する #1104

Conversation

berryzplus commented Nov 30, 2019

PR の目的

カテゴリ

PR の背景

PR のメリット

PR のデメリット (トレードオフとかあれば)

PR の影響範囲

関連チケット

参考資料

AppVeyorBot commented Nov 30, 2019

beru commented Nov 30, 2019

berryzplus commented Nov 30, 2019

berryzplus commented Nov 30, 2019

AppVeyorBot commented Nov 30, 2019

berryzplus commented Nov 30, 2019

azure-pipelines bot commented Nov 30, 2019

berryzplus commented Dec 1, 2019

azure-pipelines bot commented Dec 1, 2019

berryzplus commented Dec 1, 2019

azure-pipelines bot commented Dec 1, 2019

beru commented Dec 1, 2019

berryzplus commented Dec 1, 2019

beru commented Dec 1, 2019

berryzplus commented Dec 4, 2019

AppVeyorBot commented Dec 7, 2019

berryzplus commented Dec 7, 2019

beru commented Dec 7, 2019

beru left a comment

Choose a reason for hiding this comment

berryzplus commented Dec 7, 2019

berryzplus commented Dec 7, 2019

beru commented Dec 8, 2019

k-takata commented Dec 8, 2019

beru commented Dec 8, 2019

berryzplus commented Dec 8, 2019

berryzplus commented Dec 8, 2019

beru commented Dec 8, 2019

berryzplus commented Dec 8, 2019