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

指定した検索条件を履歴に残したい (PatchUnicode-1046 各履歴のお気に入り) #1640

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

dep5
Copy link
Contributor

@dep5 dep5 commented Apr 22, 2021

PR の目的

何度も検索すると検索条件が検索履歴から消えてしまいます。
そうならないように、また使いたい検索条件を保護するために

Mocaさん作の
PatchUnicode-1046 各履歴のお気に入り  を適用します

カテゴリ

  • 機能追加
  • 仕様変更

PR の背景

検索や置換の履歴がいっぱいになると必要な検索語句も履歴からいずれ消えてしまいます。
履歴に残すために、お気に入り機能を使います

PR のメリット

  • 使いたい検索条件が履歴からあふれて消えてしまう
  • 正規表現を試行錯誤していると、期待通りに動く検索条件がどれかわからなくなる
  • 非常に長い検索条件で末尾が確認できなくてわからなくなる。

お気に入りに入れることでそれを検索履歴に残し続けます。
検索、置換、GREPファイル、GREPフォルダ、コマンド、カレントディレクトリ
以上のタブで同様のことが可能です。

PR のデメリット (トレードオフとかあれば)

Grepダイアログには「除外ファイル」、「除外フォルダ」もありますが
これには対応していません。
オリジナルで対応しているものだけです。

お気に入りに指定できるのは25個までです。
コマンド、カレントディレクトリは27個までです。

お気に入りに指定した分だけ履歴として使える個数は減るので注意してください。

仕様・動作説明

ファイル・フォルダにはお気に入りに入れることで履歴に残すことができましたが、
この機能を検索・置換・GREPファイル・GREPフォルダ・コマンド・カレントディレクトリも対象にします。
ファイル・フォルダ以外でもお気に入り機能が使えます

PR の影響範囲

履歴の最大数と同じだけお気に入りにすると、問題があるという指摘に対応するために
ダイアログでチェックを入れられるのは検索条件は25個までにしています。

テスト内容

テスト1

手順

関連 issue, PR

参考資料

https://eternalwindows.jp/control/listview/listview07.html

適用前
before
適用後
after
チェックは合計25個まで

@AppVeyorBot
Copy link

Build sakura 1.0.3696 completed (commit 0d85756bbd by @dep5)

@beru beru added enhancement ■機能追加 specification change ■仕様変更 labels Apr 22, 2021
@usagisita
Copy link
Contributor

試していないで書き込みますが、外部コマンド実行か何かで、履歴の1番目をアクティブな変数として使用しているコードがあったような気がします。
その状態で履歴をすべてお気に入りにすると、新しく追加できなくなり、現在アクティブのものが追加されずに、履歴のトップが使われてしまう、という問題がある「かも」しれないです。

@sanomari
Copy link
Contributor

sanomari commented Apr 22, 2021

ファイル履歴で使える「お気に入り」の機能を、
「検索キーワード」と「置換文字列」にも適用したい
ということでしょうか?
(Grep系の4項目が対象?Clearってどこから出てきたの?)

現状の説明では、何をどうしたいのかがほとんど読み取れないので、そこを教えてほしいです。

既存パッチ取り込みとしてやるから焦点がボケるんでは?と思いました。

@AppVeyorBot
Copy link

Build sakura 1.0.3700 completed (commit 3bbac4eafc by @dep5)

@AppVeyorBot
Copy link

Build sakura 1.0.3701 completed (commit 97c52304aa by @dep5)

@dep5
Copy link
Contributor Author

dep5 commented Apr 23, 2021

#1639 でアドバイスをいただいた、誤ったAuthorに関連付けされていたのを直しました。

@dep5
Copy link
Contributor Author

dep5 commented Apr 23, 2021

sanomariさん
繰り返し検索をして履歴があふれて必要な検索キーワードが消えてしまったり、
しばらくたってからあの検索キーワードなんだっけ、
となって困ったことはないでしょうか。
わたしは以前検索を繰り返すマクロなどを使っていてたびたび失敗してきましたので
困っていたところこのパッチを見つけて使ってきました。

それからは本体側のコード変更を追いかけていただけなので、現在必要かどうかわかりません。
Grep除外ファイル・フォルダについては途中まで書いてみましたが、動かなかったので反映していません。

使用例です。
正規表現を使っているなら、コメントで名前を付けるとわかりやすいです
fav dialog

@dep5
Copy link
Contributor Author

dep5 commented Apr 23, 2021

usagisitaさん
履歴のトップが使われてしまう、という問題がある「かも」というのは
入力した文字が無視されるかも、ということでしょうか。
sakura_core/dlg/CDlgExec.cppのことでしょうか。

外部コマンド実行・ファイル名を指定して実行で
あふれるまで履歴を埋めてすべてお気に入りにして、新たにコマンドを入力して実行してみましたが、
入力したコマンドが実行されました。
しかし、確かに履歴に新しく追加できなくなりました。
こういう問題もあるんですね。

@ghost
Copy link

ghost commented Apr 23, 2021

dep5さん、困っていることは(少なくとも自分は)わかりました。
やりたいことも「Grepの履歴をお気に入りに登録しておきたい」ということだと自分は理解しています。

こんなことを書ける立場じゃないとは思いましたが、 #1622#1639 でも感じられた、次の点を指摘しておきます。

  1. PRの背景に挙げられているのは変更による「メリット」だと思います。
  2. PRのメリットに挙げられているのは「背景(=変更したい理由)」だと思います。
  3. パッチの変更内容を説明する必要があると思います。
  4. 古いパッチを無変更で取り込めるとは思えません。パッチに対して加えた変更、もしくは取り除いた変更などがあれば説明してください。
  5. この変更で困っていることが解消したことを、どうすれば確認できるか説明してください。
  6. この変更で新たに別の問題が発生するかもしれませんので、変更の影響を受ける機能などがあれば列挙してください。

#1640 (comment)
こういうこともあるので、単純な取り込みはできないはずです。

@dep5
Copy link
Contributor Author

dep5 commented Apr 24, 2021

オリジナルからの変更内容は以下の通りです

v0.2 バージョン番号を最新に変えた
v0.3 #1255 で追加された入力欄の下矢印で出てくるリストから
Deleteキーで直接履歴を消す機能でもお気に入りが効くようにした

@dep5
Copy link
Contributor Author

dep5 commented Apr 24, 2021

パッチを適用したビルドを数年使用してきて便利なので投稿してみただけですので
あまり詳しいことはわかりませんが少し書き直してみました。

@berryzplus
Copy link
Contributor

パッチを適用したビルドを数年使用してきて便利なので投稿してみただけですので
あまり詳しいことはわかりませんが少し書き直してみました。

ようやく分かってきました。そういうこと(c++開発者ではない)っすね。
プログラムは本来、英語が読めればなんとなく読み解けるものですよね。
c/c++に詳しい人ばかり相手にしていてそのことを忘れていました。

それなら、誰かが引き継いで機能取込までもっていく方向ではどうでしょうか?

現状が既に実績のあるマージコードであってもベースが色々変わってるので、追加の修正やら確認がいると思っています。

@ghost
Copy link

ghost commented Apr 25, 2021

パッチを適用したビルドを数年使用してきて便利なので投稿してみただけですので
あまり詳しいことはわかりませんが少し書き直してみました。

ようやく分かってきました。そういうこと(c++開発者ではない)っすね。
c/c++に詳しい人ばかり相手にしていてそのことを忘れていました。

この点気が付けなかったです。申し訳ありませんでした。

@ghost
Copy link

ghost commented Apr 25, 2021

dep5さん、誤った対応をしてしまい申し訳ありませんでした。

Grepの除外ファイル・フォルダには対応できていません

軽く調べたところ、これらの項目は「履歴とお気に入りの管理ダイアログ」でユーザーが管理することはできませんが、履歴自体は内部で持っています。
これらの項目の中からお気に入りに設定できるようにするには、ダイアログ項目の追加が必要になりそうです。

@dep5
Copy link
Contributor Author

dep5 commented Apr 25, 2021

kazasakuさん
わかりやすい書き方教えてもらってよかったです。ありがとうございました。

この機能を追加してもよいとなったら、あとはできたらみなさんにおまかせしたいです。

@dep5 dep5 marked this pull request as ready for review April 25, 2021 12:51
@berryzplus
Copy link
Contributor

berryzplus commented Apr 26, 2021

履歴の性質が異なるので、単純に「お気に入り」を有効にする対応ではマズそうに思います。

項目 追加タイミング 削除タイミング
ファイル履歴 ファイルを開いたとき(開いた後に登録しようとする) 件数超過で古いものから削除、お気に入りダイアログで削除(一度お気に入り登録したものはお気に入りダイアログ以外で削除できなくなる)
検索履歴 検索するとき(検索する前に登録し、登録した検索条件で検索する) 件数超過で古いものから削除、お気に入りダイアログで削除、検索コンボで削除

ファイル履歴は、新しく追加できなくなっても困らないです。新しい履歴が登録できないだけなので。
検索履歴は、新しく追加できなくなると困ります。新しい検索条件で検索できなくなるからです。
人によっては困らないかも知れませんが、普通は困るような気がします。

取込しようとしているパッチでこのあたりの仕様を考慮できているようには見えないので、
仕様変更提案は「一旦却下」が妥当かと思います。

@dep5
Copy link
Contributor Author

dep5 commented May 1, 2021

sakura.iniを編集して、MAXの履歴をすべてお気に入りに入れた状態でテストしてみました。
コマンド・カレントディレクトリについては最大32個・そのほかは30個設定できるようです。
Grep,Grep置換,検索,置換,外部コマンド実行について、
それぞれのダイアログを表示して入力して操作する分には通常通りに動きます。
Grep,Grep置換,検索,置換については同じタブやウインドウなら、もう一度ダイアログを表示すると、
同じ条件で検索・置換などができました。
外部コマンド実行については入力して実行自体は可能ですが
再度ダイアログを表示すると条件が2つともリセットされるようです。

しかし、ツールバーに追加した検索ボックスを使った場合は
入力した文字が無視されて履歴で検索されてしまって検索ができなくなりますね。
これがusagitaさんの指摘だと思います。

履歴に追加されなくても実行できるなら、それでいいかなと思っていましたが
これは困りますね・・・

@dep5
Copy link
Contributor Author

dep5 commented May 1, 2021

「履歴とお気に入りの管理」でしかお気に入りは設定できないのだから
削除もこのダイアログでしかできないようにするのがいいかなと思いましたが
コンボボックスから簡単に削除できる機能があったほうがいいのでrevertしました。

@sonarcloud
Copy link

sonarcloud bot commented May 1, 2021

@AppVeyorBot
Copy link

Build sakura 1.0.3720 completed (commit 9fb1b4f07c by @dep5)

@sonarcloud
Copy link

sonarcloud bot commented Jan 1, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

7.5% 7.5% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.3967 completed (commit 583d529c41 by @dep5)

@ghost ghost self-assigned this Jan 2, 2022
@dep5
Copy link
Contributor Author

dep5 commented Jan 4, 2022

#1640 (comment)
のその後です。

sakura_core/dlg/CDlgFavorite.cppのCDlgFavorite::OnNotify(NMHDR* pNMHDR)に

			case NM_CLICK:
				{
					LVHITTESTINFO lvht = { 0 };
					::GetCursorPos(&lvht.pt);
					::ScreenToClient(hwndList, &lvht.pt);
					ListView_HitTest(hwndList, &lvht);
					if (lvht.flags & LVHT_ONITEMSTATEICON) {
						ListView_SetCheckState(hwndList, (int)lvht.iItem, true);
					}
				}
				 return true;

というコードを追加するとチェックボックスをOFFにできるが、ONにできないようになります。
これだと、チェックボックス以外の場所をクリックしても反応してしまうので

					if (lvht.flags & LVHT_ONITEMSTATEICON && !(lvht.flags & LVHT_ONITEMLABEL)) {

このように変更します。
あとはチェックボックスをクリックした直前のチェックボックスONの個数がお気に入りの最大数を超えた場合にのみ動くようにします。
ダブルクリックでも反応してしまうので、ダブルクリックに同様のコードを追加し、
スペースキーを押すことでもチェックができるので、それにも対応しています。

クリック動作が反映される前にメソッドでチェックボックスをONにすることで
ONのチェックボックスをクリックしたことになり、ONが消えてしまうのかな、と想定しています。
現在私の環境では動いていますが、将来的に動作が変わってしまうかもしれません。
その場合は、お気に入り数の上限設定が働かなくなると思います。

@dep5 dep5 changed the title PatchUnicode-1046 各履歴のお気に入り 指定した検索条件を履歴に残したい (PatchUnicode-1046 各履歴のお気に入り) Jan 4, 2022
@berryzplus
Copy link
Contributor

ぼく個人の主観的な見解ですが、

リストビューの項目がクリックされた場合、
項目をチェック状態にする処理

の正当性を疑っています。

チェック付きの項目をクリックしたときに行うことは2つです。
・項目のチェック状態が変更されたことを記録する
・項目を再描画する

チェックボックスを直接触る処理を書いてしまうと、状態の記録が漏れる要因になります。
状態の記録が漏れると再描画でチェックが消えたり復活したりという微妙なバグが発生します。。。

@dep5
Copy link
Contributor Author

dep5 commented Jan 10, 2022

berryzplusさん

https://eternalwindows.jp/control/listview/listview07.html
を参考にしたのですが、

・項目のチェック状態が変更されたことを記録する

この動作には遅延があるようで、その間にチェックを入れることで
ONにしてOFFにしてしまおうというコードです
いろんな間隔でダブルクリックしてみると、ちゃんと制限できるものの
一瞬チェックが入る描画も確認できます。

スペースキーを押しっぱなしで高速にチェックしたり、
チェックを入れたりテストしてみた範囲では
期待通りに動いていると判断しています。

チェックの状態が履歴に反映されるタイミングを考えても、
再描画が完了たチェックの見た目のままお気に入りに設定されるはずです

しかし、たまたま見つけた裏技のようなものです。
検索条件を残したい人がそれでもよければ使ってみてください。

@ghost ghost removed their assignment May 5, 2022
@dep5
Copy link
Contributor Author

dep5 commented May 31, 2022

履歴最大値までお気に入りに設定した場合に問題があると指摘があったのでrevertしましたが
履歴最大値までお気に入りに設定できないように対処したので
また適用しておきます。
コンフリクトも修正しました。

@AppVeyorBot
Copy link

Build sakura 1.0.4137 completed (commit 3893a185c6 by @dep5)

@dep5
Copy link
Contributor Author

dep5 commented Jun 1, 2022

ほかの人のコミットが大量に混入したコミットになってしまったのでやり直しました

@AppVeyorBot
Copy link

Build sakura 1.0.4140 completed (commit c68c0839f9 by @dep5)

@sonarcloud
Copy link

sonarcloud bot commented Jun 10, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

7.1% 7.1% Coverage
0.0% 0.0% Duplication

@AppVeyorBot
Copy link

Build sakura 1.0.4146 completed (commit 2072ee9931 by @dep5)

@AppVeyorBot
Copy link

Build sakura 1.0.4214 completed (commit da0a17384c by @dep5)

@sonarcloud
Copy link

sonarcloud bot commented Dec 16, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

7.1% 7.1% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ■機能追加 specification change ■仕様変更
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants