Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grep 置換ダイアログを表示すると,置換前 と 置換後の両方が選択状態になる現象が発生しないように修正 #1719

Merged

Conversation

beru
Copy link
Contributor

@beru beru commented Aug 28, 2021

PR の目的

#1717 で報告された問題のうち、Grep置換ダイアログ表示した際に置換前と置換後の両方が選択状態になる現象が発生しないように修正するのが目的です。

カテゴリ

  • 不具合修正

PR の影響範囲

CDlgGrepReplace::SetData において置換後のコンボボックスに項目を追加するのではなく、WM_COMMAND メッセージの通知コードが CBN_DROPDOWN の場合に呼び出される仮想関数 CDialog::OnCbnDropDown を override した CDlgGrepReplace::OnCbnDropDown で置換後のコンボボックスの項目を追加するようにしました。

新しく追加した CDlgGrepReplace::OnCbnDropDown の処理内容は CDlgGrep::OnCbnDropDown と同じように、まだコンボボックスに要素が追加されていない場合に設定するようにしています。

テスト内容

テスト1

手順

  1. Grep 置換で何か置換を実行する
  2. 再び Grep 置換ダイアログを表示すると,置換前 だけが選択状態、置換後は選択状態ではない

関連 issue, PR

#1717

参考資料

@beru beru added the 🐛bug🦋 ■バグ修正(Something isn't working) label Aug 28, 2021
@sonarcloud
Copy link

sonarcloud bot commented Aug 29, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.3902 completed (commit 111555f701 by @beru)

@berryzplus
Copy link
Contributor

何がいけなかったんですかね?

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

問題だ、と言われてる内容は直りそうです。

コードみて気付いたどうでもいい話。
・エディタ間の履歴同期ができなくなりそうです。
・SonarScanに「1caseならswitch文使うな」、「switchにはdefault書けよ」と言われとります。

@beru
Copy link
Contributor Author

beru commented Sep 3, 2021

何がいけなかったんですかね?

根本的な原因は理解していません。自分は Windows Internals には詳しくなく、また comctl32.dll のコードを閲覧&ビルド&デバッグ&リバースエンジニアリングは出来ないので。

問題の発生にはきっと一定の条件があると推測します。CDlgGrepReplace::OnInitDialog においてフォント設定する処理をコメントアウトして無効化すると問題の現象が発生しない事は確認しました。ただフォント設定自体は保ちたいのでこの記述を無効化するわけにはいきません。

どうして同じような画面構成のGrepダイアログでは問題の現象が起きないのかコードを調べてみたところ、コンボボックスに項目を追加するタイミングが SetData メソッドではなく OnCbnDropDown メソッドで行っていました。Grep置換ダイアログの処理もそれに合わせて同じ構成にしたところ問題が発生しなくなりました。

結果オーライの対症療法です。

@beru
Copy link
Contributor Author

beru commented Sep 3, 2021

・エディタ間の履歴同期ができなくなりそうです。

これはよくわかっていません。問題があれば誰かがIssueを作成するかもしれないですね。

・SonarScanに「1caseならswitch文使うな」、「switchにはdefault書けよ」と言われとります。

書いていませんでしたが、CDlgGrepReplace::OnCbnDropDown の記述でCode Smellsが2件出ていることは確認済みです。CDlgGrep::OnCbnDropDown の記述に合わせるほうが良いだろうと判断し、今の記述のままにしています。

@beru
Copy link
Contributor Author

beru commented Sep 3, 2021

レビューありがとうございました。Mergeします。
もし問題が有れば別のPRで対処します。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants