コンボボックスのサブクラス化処理を簡素化する

[kengoide]

PR の目的

#1311 で要望されている機能の実現に向けた関連コードの整理です。
Windows Vista までに導入された API を活用します。

カテゴリ

• リファクタリング

PR の背景

検索ダイアログ等のコンボボックスに単語削除機能の追加を試みる途中、既存のコンボボックスのサブクラス化処理が幾分古く冗長であることが気になりました。
単語削除を追加すると関連コードがより複雑になるため、先に整理しておくことで以後のレビュー負担が軽減できるかと思います。

PR のメリット

• コンボボックスのサブクラス化に関わるコードの複雑度が減ります。
• SetComboBoxDeleter の呼び出し時に構造体を用意する必要がなくなります。

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

ないはずです。

仕様・動作説明

• GetComboBoxInfo を使う
  既存コードでは Edit と ListBox のハンドルを取得するために WM_CTLCOLOR* を目的外利用していましたが 、同じ情報がこの関数で取得できるためハックを除去します。

• SetWindowSubclass を使う
  元の WNDPROC を保存・リストアするコードが除去できます。

• SComboBoxItemDeleter 構造体の廃止
  構造体のメンバが HWND と CRecent* のみになったため、プロシージャの引数（HWND）とウィンドウプロパティ（CRecent*）で代替してしまいます。

テスト内容

検索ダイアログ等のコンボボックスの既存動作が変更されないことを手動で確認します。
サブクラス化で実現している履歴の削除動作に対して特に確認が必要です。

PR の影響範囲

SetComboBoxDeleter を使用する各ダイアログに影響しますが、動作は変更しません。

関連 issue, PR

#1311

参考資料

後ほど PR 予定の完成形

[AppVeyorBot]

✅ Build sakura 1.0.3247 completed (commit 35c43b51b5 by @kasumikagari)

[beru]

SetComboBoxDeleter メソッドを呼び出す際に GetItemHwnd メソッドを呼び出してコントロールのHWNDを取得して渡している記述が結構有るのが気になりました。GetItemHwnd メソッドを内部で呼び出す SetComboBoxDeleter メソッドのオーバーロードを追加するのはどうでしょうか？

[kengoide]

SetComboBoxDeleter は static なのに対して GetItemHwnd は non-static であるためオーバーロードはできません。
本質的には CDialog とは関係のない関数ですから独立させてしまって、CDialog 専用のヘルパー関数を追加するほうが良いかなと思います。

[berryzplus]

まず、趣旨には賛成です。

このPRによって、少なくとも利用側のコードは簡素化できているように見えました。
逆に、共通コード部分の複雑度が高まったように見えていて、全体把握ができてる自信がないです。

現状でOK/NGを判断できそうにないので、一旦コメントだけ残しておきます。

[AppVeyorBot]

✅ Build sakura 1.0.3250 completed (commit b6f00d71e7 by @kasumikagari)

[berryzplus]

一通り内容を確認して、問題ないかな～という結論に至りました。

先に書いていた「共通関数が複雑になった気がする」の改善点をあげてみたのでご検討ください。

理由があれば対応なしも可だと思います。
そうであれば、その旨コメントを。

[AppVeyorBot]

✅ Build sakura 1.0.3259 completed (commit 86668e0f33 by @kasumikagari)

[berryzplus]

@sakura-editor/sakura-developers
3回目PRなのでinvite出しちゃいましたが、
既にapprove済なので何かあれば早めのレビューをお願いします。

[kengoide]

丁寧なレビューをありがとうございました。
他にコメントがなければ今夕ぐらいにマージしたいと思います。