GREPの「カレントフォルダーが初期値」に対応 #1852
GREPの「カレントフォルダーが初期値」に対応 #1852
Conversation
|
✅ Build sakura 1.0.4144 completed (commit 41710c4828 by @dep5) |
を消しましたが、心配になったので元に戻しました。 まで移動してしまったので削除しました。 |
|
|
|
✅ Build sakura 1.0.4147 completed (commit 5e2c6f85bd by @dep5) |
dep5
force-pushed
the
FixGrepDefaultFolder
branch
from
3eef9f1 to
721e7b9
Compare
|
元からあるコードの条件を変更しないように書き直しました。 |
|
✅ Build sakura 1.0.4148 completed (commit 2aa1cee79d by @dep5) |
sakura_core/dlg/CDlgGrep.cpp
Outdated
| m_szCurrentFilePath[0] != L'\0' ){ | ||
| WCHAR szWorkFile[MAX_PATH]; | ||
| SplitPath_FolderAndFile( m_szCurrentFilePath, m_szFolder, szWorkFile ); | ||
| }else | ||
| if( m_szFolder[0] == L'\0' && m_pShareData->m_sSearchKeywords.m_aGrepFolders.size() ){ |
There was a problem hiding this comment.
else と if の行は分けずに else if と書いた方がコードが読み易いと思います。
There was a problem hiding this comment.
force-pushしたのですが、wcscpyの部分でSecurity Hotspotsがでてしまいました。
以前、修正で苦労したのでそれよりはもとのコードを変更しないように書き換えたら
Security Hotspotsが消えるかも、と変な改行が入っています。
There was a problem hiding this comment.
Security HotspotsってSonarCloudのでしたっけ？
静的解析のfalse positiveな警告を潰す為にコードを読みにくくするのは、本来はコード品質を良くするためにツールを使っているのに、そのツールの都合に合わせる為にコードの可読性を悪くしている事になります。それだと人間がツールに振り回されてしまっているようで、トータルとしてツールを使う事でプラスになったんだろうか？と疑問を感じてしまいます。。
sakura_core/dlg/CDlgGrep.cpp
Outdated
| @@ -250,11 +250,20 @@ int CDlgGrep::DoModal( HINSTANCE hInstance, HWND hwndParent, const WCHAR* pszCur | |||
| m_bGrepOutputBaseFolder = m_pShareData->m_Common.m_sSearch.m_bGrepOutputBaseFolder; | |||
| m_bGrepSeparateFolder = m_pShareData->m_Common.m_sSearch.m_bGrepSeparateFolder; | |||
|
|
|||
| if( pszCurrentFilePath ){ // 2010.01.10 ryoji | |||
There was a problem hiding this comment.
ryojiさんは今のプロジェクトに参加してないので解放してあげたい気がします。
ソースコードに名前が入っていると、なんか問題あったら「そいつのせい」に見えます。
There was a problem hiding this comment.
m_szCurrentFilePathを使いたかったので移動しましたが、必要なくなりました。
sakura_core/dlg/CDlgGrep.cpp
Outdated
| if( m_szFolder[0] == L'\0' && m_pShareData->m_Common.m_sSearch.m_bGrepDefaultFolder && | ||
| m_szCurrentFilePath[0] != L'\0' ){ | ||
| WCHAR szWorkFile[MAX_PATH]; | ||
| SplitPath_FolderAndFile( m_szCurrentFilePath, m_szFolder, szWorkFile ); |
There was a problem hiding this comment.
「カレントフォルダを初期値」は
Grepダイアログの次回表示時のフォルダの初期値をカレントフォルダにするだと思うので、
「初期化専用プロパティ」と考えてよいと思います。
チェックOFF/ONで制御するのは、あくまで「初期表示」の挙動なので、
Grepダイアログ表示後のチェックOFF/ONで反応しなくても問題はないです。
挿入位置はOnInitDialogにしたほうが良いと思います。
| 関数名 | タイミング |
|---|---|
CDlgGrep::DoModal |
Grepダイアログ「表示前」 |
CDlgGrep::OnInitDialog |
Grepダイアログの「初期表示時」 |
There was a problem hiding this comment.
DoModalの時は移動のほかに少し変更がいりましたが、
OnInitDialogにすると、移動するだけで済んで、すっきりしたコードになりました。
|
#1852 (comment) |
|
✅ Build sakura 1.0.4150 completed (commit d634590c7e by @dep5) |
sakura_core/dlg/CDlgGrep.cpp
Outdated
| m_szCurrentFilePath[0] != L'\0' | ||
| ){ | ||
| WCHAR szWorkFile[MAX_PATH]; | ||
| SplitPath_FolderAndFile( m_szCurrentFilePath, m_szFolder, szWorkFile ); |
There was a problem hiding this comment.
SonarCloudはCスタイル配列を使っても警告出さなくなったんですね。
SplitPath_FolderAndFileの第3引数は nullptr を受け入れるので szWorkFile のメモリ確保は無駄です。
500バイトくらい許容だと思いますが、文句言う人がいそうです。（既知のGuys...）
There was a problem hiding this comment.
別観点ですが、
OnBnClicked(IDC_BUTTON_CURRENTFOLDER);
とかにしなくて良いのかな？（無題の場合どうなる？）ということを思いました。
これ、指摘じゃないです。
There was a problem hiding this comment.
削除部分のコメントしてるのは間違いだと思いましたが、意味があったんですね。
よく読めばよかったです。
|
Kudos, SonarCloud Quality Gate passed! |
|
✅ Build sakura 1.0.4152 completed (commit 0a8716dfd9 by @dep5) |
PR の目的
#1851 で報告された問題に対応します
カテゴリ
PR の背景
#1696 により動かなくなっています。
PR のメリット
PR のデメリット (トレードオフとかあれば)
仕様・動作説明
CDlgGrep::SetData()の中で"カレントフォルダーを初期値"がONの場合
が動きますが、#1696 での追加で
別の変数で上書きしています。
https://github.com/sakura-editor/sakura/releases/tag/v2.4.1
で動きを確認してみると、
チェックボックスをONOFFするたびに反映されるタイプのものではなく、
ダイアログ表示時に一度動けばよいようです。
CDlgGrep::SetData()からCDlgGrep::DoModal()へ移動して対応してみました。PR の影響範囲
テスト内容
テスト1
#1851
「カレントフォルダーを初期値」にチェックを入れた状態で「検索」を押した後
ファイルを開いてからGREPダイアログを表示します。
手順
関連 issue, PR
#1851
#1696
参考資料