DoGrep() 引数の構造体化対応 - #2634
Conversation
|
|
自分も何度かやろうとして何度か断念してるやつです。 データの集約先はGrepInfoでよいと思います。 コマンドラインからGrepInfoを作ってCDlgGrepに渡す 本当はたぶん、以下が理想形。（現状は無理。） コマンドラインから取り込んだ引用符の二重化を解除する目的で CNativeW を使っているけど、CNativeWとダイアログ入力の文字数制限の相性があまりよくない。 それはそれとして、内容的に問題はなさそうなのでそのうちマージする感じだと思います。 |
|
ご確認ありがとうございます。 |
| gi.bGrepPaste, | ||
| gi.bGrepBackup | ||
| ); | ||
| this->m_pcEditApp->m_pcGrepAgent->DoGrep( &pEditWnd->GetActiveView(), gi ); |
There was a problem hiding this comment.
後回しでいいですが、DoGrepの既存パラメーターは以下のような分類だったと思ってます。
- Grepダイアログ・コマンドライン共通のパラメーター
- コマンドライン用のパラメーター（bGrepStdoutとbGrepHeader）
- Grep置換ダイアログのパラメーター（bGrepPasteとbGrepBackup）
- 不要なパラメーター （CEditViewのポインタ）
※「アクティブな編集ビュー」はプロセス内に1つしか存在できない「シングルトン的なオブジェクト」なので、指定しなくても呼ばれた側で取得できます。むしろ「左右分割したビューの右側だけに結果表示」とかできないので指定させることがおかしい気もします。（この件は「全体的に」なので「超あとまわし」でよいはず。）
「渡したパラメーターが見える」ほうが分かりやすいよね、で
GrepInfo への集約とは別枠で「あえて外出しにするパラメーター」があってもいい気がします。
細かいことは「今後」でいいのかな、と。
| @return 出力・置換の挙動を決めるオプション | ||
| @note Grep置換では「一致しなかった行を出力」が成立しないため、行単位出力に落とす。 | ||
| */ | ||
| SGrepOption SGrepOption::FromGrepInfo( const GrepInfo& gi ) |
There was a problem hiding this comment.
現状 既に CDlgGrepReplace inherits CDlgGrep です。
今後 CDlgGrep inherits GrepInfo にできたらいいな、と思っています。
GrepInfo inherits SGrepOption にするかどうかは悩みドコロ。
結局、CGrepAgent::DoGrep の実行に必要なパラメーターが揃っていることを担保する責任は誰にあるのか、揃ってなかったらUI表示して入力要求するのがスジなんじゃないか、みたいな厄介なことを色々思ってます。
| // (正規表現が途中で途切れると困るので) | ||
| // 2011.12.10 Moca 表示の際に...に切り捨てられるので登録するように | ||
| wcsncpy_s( CAppMode::getInstance()->m_szGrepKey, int(std::size(CAppMode::getInstance()->m_szGrepKey)), pcmGrepKey->GetStringPtr(), _TRUNCATE ); | ||
| wcsncpy_s( CAppMode::getInstance()->m_szGrepKey, int(std::size(CAppMode::getInstance()->m_szGrepKey)), gi.cmGrepKey.GetStringPtr(), _TRUNCATE ); |
There was a problem hiding this comment.
_TRUNCATEってことはさ。
入り切らなかったらブッタ斬ります。
ってことなのよね。（いいんだっけ。）
（ただのポエム。）
|
マージいただきありがとうございました。 _TRUNCATE はm_szGrepKey は StaticString<MAX_GREP_PATH> なので
いずれも表示専用で、実際の検索は gi.cmGrepKey を使っています。512 文字を超えて |
2 participants
PR #2631 の続きで、
CGrepAgent::DoGrep()の 20 個の引数を構造体渡しに変更してます。変更内容
DoGrep()をDWORD DoGrep( CEditView* pcViewDst, const GrepInfo& gi )の 2 引数にしました。DoGrep()冒頭の「Grepオプションまとめ」18 行をSGrepOption::FromGrepInfo()に切り出しました。GrepInfoを作るCDlgGrep::MakeGrepInfo()/CDlgGrepReplace::MakeGrepInfo()を追加しました。CNormalProcess::InitializeProcess()、Command_GREP()、Command_GREP_REPLACE()）を書き換えました。呼び出し側への影響
Command_GREP()は実引数を組み立てるためのcmWork1〜cmWork4が不要になったので削除しました。Command_GREP_REPLACE()の同名変数は else 側でコマンドライン文字列を作るのに使っているため残しています。従来ダイアログ経路が固定値で渡していた
bGrepCurFolder = false/bGrepStdout = false/bGrepHeader = trueはMakeGrepInfo()の中で同じ値を設定しています。テスト
tests1 は 1,363 件 / 0 failed（#2631 の 1,357 件 +6）、SonarQube の New Code カバレッジは 87.3%、重複 0.0% です。
意図的にやっていないこと
DoGrepTree()/DoGrepFile()/DoGrepReplaceFile()の引数整理。本 PR はDoGrep()の入口だけに絞っています。BOOLの一括bool化。PR Grepのファイル一覧処理から手動メモリ管理を削除 #2631 でご指摘いただいた件ですが、利用箇所の全数確認が必要なので別途とします。LPCWSTR→std::wstring_view移行。Issue ご意見をお願いします。PR #2524 分割対応案 #2596 の横断課題として扱います。確認したいこと
CDlgGrepにMakeGrepInfo()を足した形でよいでしょうか。呼び出し側で
GrepInfoを組み立てるとそこは単体テストから叩けず、New Code カバレッジが 46.6% でした。ダイアログ側に寄せると直接構築してテストでき 87.3% になっています。CDlgGrep::GetData()まわりの整理でまとめて扱うべきであれば、本 PR からは外して呼び出し側に戻します。