ファイル名の簡易表示設定での項目未選択時における更新処理の改善 #1478
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ Build sakura 1.0.3279 completed (commit 06608d10ad by @kazasaku) |
berryzplus
reviewed
Dec 6, 2020
sakura_core/String_define.h
Outdated
| @@ -713,6 +713,7 @@ | |||
| #define STR_PROPCOMFNM_LIST1 34528 | |||
| #define STR_PROPCOMFNM_LIST2 34529 | |||
| #define STR_PROPCOMFNM_ERR_REG 34530 | |||
| #define STR_PROPCOMFNM_WARN_NOSEL 35043 | |||
There was a problem hiding this comment.
指摘とすべきかどうか超悩むところなんですが、ここに追加したいです？
代替候補はsakura_rc.h、普通はこっちに追加する気がします。
なんだけど、正直どっちでも構わないです。
There was a problem hiding this comment.
現状F_*やSTR_*がここに纏めてあるようなのでここに書きました。
berryzplus
reviewed
Dec 6, 2020
| @@ -238,9 +238,13 @@ INT_PTR CPropFileName::DispatchEvent( HWND hwndDlg, UINT uMsg, WPARAM wParam, LP | |||
| }else{ | |||
| // 未選択でリストにひとつも項目がない場合は追加しておく | |||
There was a problem hiding this comment.
何を？・・・何を追加しておくつもりだったんですかね、元のコメントは。
このコメントが正しいと仮定すると、ぼくらがGitHub移行後に壊した可能性がちょっと出てくるわけです。
There was a problem hiding this comment.
誤解のないようにコメントを書き直したほうがよいと思います。
あと、else句を追加してメッセージ出すように変えてますが、ifの中にも前処理が入っているのはなんででしょう？なんとなく、ifの中の前処理が「本来あるべき初期化」にあたるもので、「ぬけてたのを補う」がやるべきことなのかな？と思いました。・・・で、そうするとelse句は追加要件（？）にあたるので一緒にやるべきでないような。
There was a problem hiding this comment.
blameしたところ、かなり昔からこうなっているみたいでした。
dc9ce65
ghost
closed this
|
すみません、間違ってCloseしてしまいました。 |
ghost
mentioned this pull request
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR の目的
「ファイル名の簡易表示」設定画面にある更新ボタンに存在するバグを修正します。
カテゴリ
PR の背景
SonarQubeのissueを眺めていたところ、気になるものがあったので対応しようと思いました。
The right operand of '==' is a garbage value
指摘のある関数の呼び出し元のうち、更新ボタンのイベント処理から呼ばれるルートの時で、文字列の取得をしないまま（文字列が格納される変数を初期化しないまま）当該関数に引数として渡していました。
PR のメリット
PR のデメリット (トレードオフとかあれば)
仕様・動作説明
共通設定ダイアログの「ファイル名の簡易表示」設定画面にある更新ボタンは、次のように動作しています。
このうち、リストが空で何も選択していない時に行われる追加処理で、テキストボックスから文字列を取得する処理が抜けていたため、これを追加しています。
（変更前は、項目を追加できていませんでした。）
また、これとほぼ同じ画面構成であるタイプ別設定ダイアログの「正規表現キーワード」と「キーワードヘルプ」の各画面では、何も選択せずに更新ボタンを押した場合はメッセージを表示するようになっていますので、それらに合わせて「リストが空でないにも関わらず何も選択していない場合はメッセージを表示する」ようにしました。
（変更前は、何も表示されていませんでした。）
PR の影響範囲
共通設定ダイアログにある、ファイル名の簡易表示設定
テスト内容
テスト1
手順