利用できないコマンドラインオプション「-WQ(INIファイルを出力して即終了)」を削除したことにより可能となったコード整理を実施する #1435
Conversation
|
✅ Build sakura 1.0.3176 completed (commit ca252e3020 by @berryzplus) |
| @@ -39,9 +39,6 @@ struct EditInfo; | |||
|
|
|||
| //! iniフォルダ設定 // 2007.05.31 ryoji | |||
| struct IniFolder { | |||
| bool m_bInit; // 初期化済フラグ | |||
| bool m_bReadPrivate; // マルチユーザ用iniからの読み出しフラグ | |||
| bool m_bWritePrivate; // マルチユーザ用iniへの書き込みフラグ | |||
| WCHAR m_szIniFile[_MAX_PATH]; // EXE基準のiniファイルパス | |||
| WCHAR m_szPrivateIniFile[_MAX_PATH]; // マルチユーザ用のiniファイルパス | |||
| }; /* iniフォルダ設定 */ | |||
There was a problem hiding this comment.
この構造体を削ってしまってはいかかがでしょう。
この構造体を残しておくことのメリデメ
| 説明 | |
|---|---|
| メリット | INIフォルダ判定結果をキャッシュできる。 |
| デメリット | 共有メモリに依存するので単体テストが書けない。 |
判定のオーバーヘッドも変数サイズもたいしたことないので、廃止して単体テストを書けるコードを増やしたほうがよさそうに思います。
There was a problem hiding this comment.
PRの一部を分割したので一旦閉じて出し直します。
There was a problem hiding this comment.
なんでですか？
具体的な理由を説明できますか？
PRを出してもレビュアーが理解できないだろう、という意味で「無理かも」と書きました。
現在の単体テスト tests1.exe には4種類のテストが含まれています。
- 本物の単体テスト （コード＋スタブ等でコードそのものの良し悪しを判断するテスト）
- 単体といいつつ、モジュールレベルの結合動作を確認するテスト （test-loadstring.cppなど）
- 単体といいつつ、システムレベルの結合動作を確認するテスト （test-winmain.cpp）
- gtest の使用例サンプル
・・・。この情報だけでもお腹一杯になれると思います。
普通に考えて、目的の異なる複数の種類のテストが1モジュールに詰まっていることは問題だと思います。
たぶん先に、テストモジュールを分割する対応が要って、そこだけで20手（≒20PR）くらい要りそうです。
事前の変更を行っている間に、ここの変更のことを覚えている自信がないので閉じずに放置してある次第です。
There was a problem hiding this comment.
なんでですか？
具体的な理由を説明できますか？PRを出してもレビュアーが理解できないだろう、という意味で「無理かも」と書きました。
誰でも理解できるようには説明できない、と解釈しました。
それは当然だと思います。
ここの実装をテスト可能にする話と単体テストの種類が混在している話は別じゃないかと思いました、テストを混在させたままでもテスト可能にする変更はできると思います。
もちろん、gmockを使うテストは不可能でしょうけど、必要なんですか？
There was a problem hiding this comment.
どうぞ。
どの範囲を？って疑問はありますが、自分がレビュー側にまわったほうがマージが早い傾向にあると思います。
（まー、レビューが適当だからなんですけども 😄 ）
There was a problem hiding this comment.
どうぞ。
どの範囲を？って疑問はありますが、自分がレビュー側にまわったほうがマージが早い傾向にあると思います。
（まー、レビューが適当だからなんですけども 😄 ）
GetInidirをプロセスがない状態でも呼び出せるように変更する修正です。
このPRよりも一回り広い範囲の修正になると思います。
…ditor#1435 利用できないコマンドラインオプション「-WQ(INIファイルを出力して即終了)」を削除したことにより可能となったコード整理を実施する
|
#1512 に完全に取り込まれるようなのでこちらは閉じてしまいます。 |
PR の目的
利用できないコマンドラインオプション「-WQ(INIファイルを出力して即終了)」を削除したことにより可能となったコード整理を実施します。
カテゴリ
PR の背景
#1432 で一度出した 14c8faf の出し直しです。
修正内容が挙動を変えないことを目視確認できるようにコミットを分けただけの代物です。
（といいつつ、内容は少し変えていますが:smiley:）
変更は個々のコミットを見たら自明となるように作ってあります。
緒tt迷った感じのコミットもいくつかあります。
PR のメリット
コードが多少分かりやすくなります。
PR のデメリット (トレードオフとかあれば)
とくにないと思います。
仕様・動作説明
仕様・動作に変更はありません。
このPRは不要となった共有メモリの構造体メンバーを削除するため、共有メモリバージョンを変更します。
テスト内容
このPRは外部仕様を一切変更しないため、エディタの起動を確認できれば十分です。
PR の影響範囲
コントロールプロセスの初期化に影響する変更です。
sakura.exe.ini を使ってマルチユーザー設定にした場合の動作に影響します。
関連 issue, PR
#1432
参考資料