[WIP] grep の除外指定の残件対応

[berryzplus]

#768
自分が理解している範囲で対応を行いました。

あんまり厳密な確認はしていませんが、なんとなくイケてそうです。

• ファイル !!! !!!.txt を除外ファイルとして認識できるか
• フォルダ ### ### を除外フォルダとして認識できるか
• Grepウインドウからの実行でそれっぽい動きをするか
• Grepウインドウ以外からの実行でそれっぽい動きをするか
• Grep置換がそれっぽい動きをするか
• 履歴登録がそれっぽい動きをするか

[berryzplus]

ちょっと考え直します。修正量もっと減らせそう。

[berryzplus]

修正方法を再考してコミット組みなおしました。
何を変えてるかは各コミットメッセージで説明しています。

[berryzplus]

後半部分にあった変数型、変数名のリファクタリングをカットしました。
リファクタリングをしなくてもプログラム動作に悪影響はないので別件とします。

ここで対応するのは実害のある部分だけにします。

[berryzplus]

レビュープリーズ

[m-tmatma]

既存の処理の何が問題でどのような方針で解決するのかの説明が欲しいです。

[berryzplus]

既存の処理の何が問題でどのような方針で解決するのかの説明が欲しいです。

問題があって直すのは「既存の処理」ではなくて、きみが #403 で入れたコードですよ。

コミットログ見たらわかると思うんですが、説明追加します。

ここが中核。 f4b9f2f

パラメータGFILEを分解する機構を実装

変更内容をGrepダイアログ(Grep置換と共用)に押し込めるように実装。
表示の直前までは旧仕様のパラメータで動き、画面表示は新仕様、履歴保存も新仕様で動く。
ダイアログが閉じられたときのハンドラでパラメータを旧仕様に変換し、爾後の処理が旧仕様のまま走るようにした。

何が問題　＝　UI追加の影響が、UIと関係ないところにバラまかれている。
改修方針　＝　変更内容をUI内部に閉じ込める。

この改修仕様で問題が出るとしたら、それは除外パターン対応よりも前からあった既存の問題と言えると思います。

[m-tmatma]

問題があって直すのは「既存の処理」ではなくて、きみが #403 で入れたコードですよ。

@berryzplus さんの要望に応えるために制限事項があるとわかりながらそのように変更しました。

現状での 「既存の処理」の何が問題であるかを説明する必要があると思います。
この PR に関しては、私以外の方がレビューしたほうがいいと考えます。

それが説明を求める理由です。

[berryzplus]

この PR に関しては、私以外の方がレビューしたほうがいいと考えます。

優先順位1番は @m_tmatma さんです。入れたPRに難癖ついてんだからauthorが見るべき。（2位は自分
@m_tmatma さんに理解できないような外れた変更なら説明もしますが、必要ないと思っています。

つーかやっぱりわからん、というなら説明資料用意しますので、ちょっと数週間待ってください。

[m-tmatma]

この PR に関しては、私以外の方がレビューしたほうがいいと考えます。

優先順位1番は @m_tmatma さんです。入れたPRに難癖ついてんだからauthorが見るべき。（2位は自分
@m_tmatma さんに理解できないような外れた変更なら説明もしますが、必要ないと思っています。

この PR は私の入れた修正をほど大部分削除して、別のコードに置き換えるものです。
もとの PR を入れるときに私のやり方に @berryzplus さんが納得していないけど
approve しマージした。

でも @berryzplus さんは自分のやり方がいいと思っているのでもともと
主張されていた方法で対処する PR を投げられている。

その状態でまた私がレビューしたとしてもまた基本的な考え方の部分での違いに
関しては別に変わるような状況変化があるわけでもないので私がレビューしても
別に同じことの繰り返しになるだけだと考えています。

その意味で第三者の目から現状 master に入っている方法の延長で対処するほうが
いいのか、あるいは この PR の方法で対処するのがいいのか検討するのがいいと
思います。

[berryzplus]

とりあえず了解。

問題あっても直せばいいと思ってるのでレビューは妥協しました。問題あっても直さないってんならレビューの仕方を考えないといかんですね(今回は良いです。

制限事項としている問題が解消できてるかどうかの判断だけなんで、この状態でもレビューは可能だと思っています。

どうですか？ > @beru さん

[beru]

• CDlgGrep::OnDestroy で m_szFile, m_szExcludeFile, m_szExcludeFolder を m_szFile に統合
• MergeAndClassifyGFile で m_szFile を分解して m_szFile, m_szExcludeFile, m_szExcludeFolder に分配

していますが、CDlgGrep がコマンドライン引数仕様従来の CGrepAgent 仕様に沿う為に無理をしているような感じがして気になります。

既存の実装が CDlgGrep::m_szFile メンバ変数を直接参照しているのでいかんともし難い感がありますが、メソッドや関数経由にする方が I/F 境界がはっきりしていて良いような気がします。リファクタ的な話題になってるかもしれませんが…。

肝心の動作確認は申し訳ないですがすぐには出来なそうです。
お詫びに遊園地のビデオでも…。

[m-tmatma]

情報共有です。
#809
を登録してますが、もしこれを対応するとなると
大きく制御を変える必要があるかもしれません。

[berryzplus]

レス返せてなかったですね。

Grep起動周りは、元がかなりごちゃごちゃしてるので、何処かで整理する必要があると思っています。

当座の対応として除外パターンのuiをより直感的にしたいってのが除外パターン対応の趣旨だと理解してます。

こらはuiの変更だと考えれば、変更箇所であるダイアログ以外に変更が入ったらいかんことになります。

除外パターン対応ではパターンごとな履歴管理にも対応してるので、変更はダイアログだけじゃないんですが、このPRを入れると最小限の変更になると思っています。

[berryzplus]

なんで放置してたか忘れるくらい放置してた感じなので、少し分かりやすい単位に分割しようと思います。

[berryzplus]

#1210 で根っこにあった問題が解決されたのでクローズします。