Sakura: 検索ダイアログボックスでプルダウンが表示時にDeleteキーで文字の消去が機能しない

Created on 29 Mar 2020  ·  24Comments  ·  Source: sakura-editor/sakura

再現手順

  1. サクラエディタを起動します。
  2. Ctrl+F で検索ダイアログボックスを表示します。
  3. 条件(＆N) に「ああああ」と入力します。
  4. 点滅カーソルを「ああ｜ああ」のように真ん中に移動させます。
  5. スクリーンショットと同じようにプルダウンを表示します。
  6. Deleteキーを押下します。
  7. 何も起きません。

期待する結果は
7.「ああ｜あ」のように1文字消去されることです。

スクリーンショット

image

プルダウンを表示した状態でBackspaceキーを押下すると
「あ｜ああ」のように1文字消去されますし、
プルダウンを表示していない状態でDeleteキーを押下すると
「ああ｜あ」のように1文字消去されます。

再現頻度

100％

問題のカテゴリ

  • 仕様の問題
  • プログラムの動作上の問題

    • 正式リリース版

    • ローカルビルド版

環境情報

  • OS バージョン
    Windows 10 Pro 64bit

    サクラエディタバージョン

  • Release版サクラエディタ Ver. 2.3.2.0

    Share Ver: 172
    Compile Info: V1500 WPR WIN500/I501/C000/N500
    Last Modified: 2017/5/2 20:58:16

  • 先ほどZipファイルでソースコードをダウンロードしたローカルビルド版サクラエディタ v2.4.0.0 32bit dev

    Compile Info: V1916 WPR WIN601/I800/C000/N601
    Last Modified: 2020/3/29 19:43:06

picture arigayas

Most helpful comment

色々試行錯誤してPRしてみました。
@arigayas さんの希望とマッチするかどうか分からんですがコンボの編集状態とdeleteされるものの辻褄が合う感じに仕様変更する形にしました。

picture berryzplus on 31 Mar 2020
❤1 👍1

All 24 comments

個別のissueにすべきかもしれませんがまとめて報告です。
チェックは上記と同じローカルビルドにて確認しました。

置換ダイアログボックス

「置換前(＆N)」「置換後(&P)」に同条件で
Deleteキーを押下した場合には何も起きませんでした。

Grep条件入力ダイアログボックス

「条件(＆N)」「ファイル(＆T)」「フォルダ(＆O)」「除外ファイル」「除外フォルダ」　の
それぞれに同条件でDeleteキーを押下した場合には何も起きませんでした。

Grep置換ダイアログボックス [検索(＆S)→Grep置換]

「置換前(＆N)」「ファイル(＆T)」「フォルダ(＆O)」「除外ファイル」「除外フォルダ」の
それぞれに同条件でDeleteキーを押下した場合には何も起きませんでした。
ただし「置換後(&Z)」だけは期待する結果の動作をしました！

arigayas picture arigayas on 29 Mar 2020

Ver.2.0.3.1 ではドロップダウン表示状態で Delete キーが機能しました。

Ver.2.3.2.0 までのどこかで Delete キーによりドロップダウンリストの項目を削除する機能が追加されているようです。たぶんその影響で Delete キーが効かないのではないでしょうか。何も考えていませんが、両立できるでしょうか？

ds14050 picture ds14050 on 29 Mar 2020

うーむ、 @ds14050 さんの言ってる「その影響」っぽいですねぇ。

ここで wParam には WM_KEYDOWN の原因となった仮想キーコードが入ってきます。
https://github.com/sakura-editor/sakura/blob/54cbf760f5ed558b23f4d17f453f0015bb6bc559/sakura_core/dlg/CDialog.cpp#L716-L722
仮想キーコードが VK_DELETE のとき、なんかしてますね・・・。

ナニをしてるんだろう？と続きを見ると、
コンボのリストが開いているかチェックしています。
https://github.com/sakura-editor/sakura/blob/54cbf760f5ed558b23f4d17f453f0015bb6bc559/sakura_core/dlg/CDialog.cpp#L722-L727
で、開いていたら DeleteItem して return 0(=終了) してるわけです。

BackSpaceキーを押したときと挙動が異なるのは、こいつが実行されないからだと思います。
https://github.com/sakura-editor/sakura/blob/54cbf760f5ed558b23f4d17f453f0015bb6bc559/sakura_core/dlg/CDialog.cpp#L742
この呼出は、windows標準のEdit実装に「あとは任せた！」ってしてる処理です。

VK_DELETEの場合、条件が揃ったらreturnしちゃうのでこれが実行されないわけで・・・。

というわけで、こいつ削ったら直るような気がします。
https://github.com/sakura-editor/sakura/blob/54cbf760f5ed558b23f4d17f453f0015bb6bc559/sakura_core/dlg/CDialog.cpp#L727

それはそうと、検索履歴をキーボードから消せる機能って必要なんですかね？

おいら、この機能のことはよく知りませんでした。

検索履歴の先頭に登録されてた正規表現が消えてなくなることがたまにあって、気にはなっていたんですけど。

ドロップダウンリストが開いているかどうかだけじゃなくって、開いたリストが選択されているかどうかも見るほうが親切な気がしました。

berryzplus picture berryzplus on 29 Mar 2020

Ver.2.0.3.1 ではドロップダウン表示状態で Delete キーが機能しました。

Ver.2.3.2.0 までのどこかで Delete キーによりドロップダウンリストの項目を削除する機能が追加されているようです。たぶんその影響で Delete キーが効かないのではないでしょうか。何も考えていませんが、両立できるでしょうか？

なるほど、そういう仕様変更があったんですね。

検索の文字列全部を選択状態ならDelete キーを押下したらドロップダウンリストの項目を削除されて、
image

「ああ｜ああ」のような状態でDeleteキーを押下したら「ああ｜あ」のように1文字消去されるようにするのは可能なのでしょうか？
image

arigayas picture arigayas on 29 Mar 2020

「置換後」は、単に実装漏れのようです。

CDlgGrepReplace::OnInitDialogにあるべき初期化関数の呼び出しがないように見えます。
構造的に CDlgGrep::OnInitDialogと似た内容の初期化が必要なはずなので。

「ああ｜ああ」のような状態でDeleteキーを押下したら「ああ｜あ」のように1文字消去されるようにするのは可能なのでしょうか？

動作確認まで終わりました。
pushしたけどコミットメッセージがテキトーすぎるからPRできねっす...orz

berryzplus picture berryzplus on 29 Mar 2020

PR出来る品質になるのを気長に待ちます😆

arigayas picture arigayas on 30 Mar 2020

色々試行錯誤してPRしてみました。
@arigayas さんの希望とマッチするかどうか分からんですがコンボの編集状態とdeleteされるものの辻褄が合う感じに仕様変更する形にしました。

berryzplus picture berryzplus on 31 Mar 2020
❤1 👍1

PRしていただきありがとうございます。

gif アニメーションで動作を確認できたら嬉しいです。
ちなみにgifアニメーションは ScreenToGif などで作成するのが良いかと。

arigayas picture arigayas on 1 Apr 2020

gif アニメーションで動作を確認できたら嬉しいです。
ちなみにgifアニメーションは ScreenToGif などで作成するのが良いかと。

ダウンロードしてgifアニメ作ろうとしてみました。
まさかの、使い方がわからんですｗ

いや、さすがに録画の仕方的なとこは分かったんですが、録画したファイルを保存できんとです。

人には得手不得手があるもんですが、おいらの不得意分野かも知んないです。

berryzplus picture berryzplus on 1 Apr 2020

GIFアニメを保存 ←ブログ記事へのリンクです。

arigayas picture arigayas on 1 Apr 2020

ScreenToGifの安全性
リンクのすぐ上に保存方法について詳細な説明が書かれてるブログ記事へのリンクです。

arigayas picture arigayas on 1 Apr 2020

動画とろうと色々試していたら意図しない動きをしたので一旦保留にします。

  1. コンボボックスにキャレットがある状態で↓キーを押下（ドロップダウンが出る）
  2. リストの項目をどれか選択（エディットに全選択状態で反映される）
  3. リストの項目を編集（エディットの全選択状態を解除）
  4. Deleteキー押下

期待値：エディット上の文字が削除されるだけ。
実際：リスト項目が削除されてしまう。

berryzplus picture berryzplus on 4 Apr 2020

あらま・・・。

arigayas picture arigayas on 4 Apr 2020

https://github.com/sakura-editor/sakura/issues/1219#issuecomment-608973068 は既に直してあって、それを踏まえた動画の編集に苦戦中です。

新旧比較したいんだけど、うまくいかぬ・・・。
（保存の仕方が分からん問題は、なんとなく自己解決できたっぽいです）

berryzplus picture berryzplus on 4 Apr 2020
👍1

Gif アニメを複数、コメントに貼ることで比較できそうですが、そういう事ではないですか？

arigayas picture arigayas on 4 Apr 2020

なんかまとまりきらないけど、とりあえず動画上げます。

sakura-find-combo5

修正前がどうなっていたかというと「ドロップダウン表示中のdeleteはリストが選択されていない限り無視」でした。

細かい挙動を見た感じ、もうちょっと小細工してもいい気もしています。

エディットでリストにある項目を入力⇨上下キー押下で選択される⇨左キーを押してもリストの選択は解除されない（このへん）

berryzplus picture berryzplus on 5 Apr 2020
👍1

PR #1221 が出されてからは傍観を決め込んでいましたが、頓挫しているのでこの機会に私の視点でおさらいをしてみようと思います。

時系列

@arigayas さんの報告に対して「何も考えていませんが、両立できるでしょうか？」と書いただけで投げ出すのは無責任だと思ったので、直後から調査を開始していました。途中経過が以下の２つのコミットです。

  1. https://github.com/sakura-editor/sakura/issues/1219#ref-commit-2d6702e
  2. https://github.com/sakura-editor/sakura/issues/1219#ref-commit-97b188b

１番目が捨てられていた Delete キーを１文字消去に利用するための変更で、２番目がそれに、ひとつだけ仲間外れだった「置換後(&Z)」を他と揃えるための変更を加えたものです。

この Issue で報告された当初の問題を解決するための変更は基本的にこれだけです(※実際にはバグがあるので少しの手直しが必要)。

その後 @arigayas さんから両立するための案として https://github.com/sakura-editor/sakura/issues/1219#issuecomment-605663852 が出されました。

そのようにしなくても両立はできたわけですが、従来の両立のしかたというのがやや微妙な挙動を含んでいました。@berryzplus さんの言葉を借りると「履歴に登録されている検索条件の一部をちょっと変えて検索したい場合に、従来の挙動だと「迂闊にDeleteキーを押せない状態」になっていました」ということです。

自分自身の問題意識と @arigayas さんの案を踏まえてさらに修正を加えたものが以下です。

  1. https://github.com/sakura-editor/sakura/issues/1219#ref-commit-18a3275

ユーザーの意識が履歴からエディットボックスに移っていると考えられる状況(※)ではエディットボックスの編集を優先し、履歴の削除(とそれに伴うエディットボックスのクリア)をしないようにしました。※具体的な認定条件は SubEditProc 関数内のコメントに書きました。

これを結論として調査を終えました。現在はこれにちょっとした操作を加えた以下の案もありますが、人によっては余計に感じるかもしれません。(何をやっているかは DeleteItem 関数内のコメントに書きました)

  1. https://github.com/ds14050/sakura-clone/compare/54cbf76...25793da

結び

2、3、4 のどれかで間に合うなら PR を出します。緑のチェックマークをクリックした先でダウンロードすれば３つとも動作の確認ができます。さらなる作り込みが必要だと考えるなら、コード片に著作権はありませんから利用できるものをすべて利用して自由にやればいいと思います。

ds14050 picture ds14050 on 9 Apr 2020

脇道それますが。

コードは著作権ありますよ。

著作権を主張しないのは個人的自由ですが、それとは無関係に、コードには著作権があります。

berryzplus picture berryzplus on 10 Apr 2020

そうでしたね。興味がないので Linux か FSF か知りませんが、パッチの提供者にも漏れなく同意を求めるとかなんとか。テキトー言いました。

ds14050 picture ds14050 on 10 Apr 2020

正確には「今後」の話をしていて、提案した実装なり仕様なりはZLib Licenseのサクラエディタに組み込まれるってことです。

他メンバーの承認を受けてマージされた瞬間、「俺のコード」じゃなく「オレ達のコード」に変わるんです。

おいらも含めて、そのことを改めて認識する必要がある気がしたので拾って見ました。

berryzplus picture berryzplus on 10 Apr 2020

PRを分けました。

1223 と #1224 を両方マージできたら、このissueをクローズできる感じです。

berryzplus picture berryzplus on 11 Apr 2020
👍1

1223 をクローズしたので焼き直しPRを準備中です。

たぶん、問題は2点。

  • 履歴コンボでドロップダウン中は、エディットで入力したDeleteキーが効かない。
  • 履歴コンボでリストをDeleteするとき、予想外のモノが消えることがある。

    • 選択されて見えるものと実際に消えるものがあってないところがある。

berryzplus picture berryzplus on 29 Apr 2020

1255 をマージしました。

  • 【問題】履歴コンボでドロップダウン中は、エディットで入力したDeleteキーが効かない。

    • 【対策】 Deleteキーが効くようにしました。

  • 【問題】履歴コンボでリストをDeleteするとき、予想外のモノが消えることがある。

    • 選択されて見えるものと実際に消えるものがあってないところがある。

    • 【対策】 エディットの内容がドロップダウンリストの選択項目と一致していて全選択になっているときだけ履歴を削除するように仕様変更しました。

動く状態のバイナリ(=sakura.exe)は https://ci.appveyor.com/project/sakuraeditor/sakura/builds/32601667/job/ais71h7o1qbiy1jp/artifacts からダウンロードできます。

問題なければ issue のクローズをお願いします　＜ @arigayas さん

berryzplus picture berryzplus on 5 May 2020 
* 【問題】履歴コンボでドロップダウン中は、エディットで入力したDeleteキーが効かない。

  * 【対策】 Deleteキーが効くようにしました。

* 【問題】履歴コンボでリストをDeleteするとき、予想外のモノが消えることがある。

  * 選択されて見えるものと実際に消えるものがあってないところがある。

    * 【対策】 エディットの内容がドロップダウンリストの選択項目と一致していて全選択になっているときだけ履歴を削除するように仕様変更しました。

動く状態のバイナリ(=sakura.exe)は https://ci.appveyor.com/project/sakuraeditor/sakura/builds/32601667/job/ais71h7o1qbiy1jp/artifacts からダウンロードできます。

希望通りの動作です。たぶん問題無いと思います。クローズします。
作成していただきありがとうございました！

arigayas picture arigayas on 5 May 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

KENCHjp picture KENCHjp  ·  31Comments
m-tmatma picture m-tmatma  ·  35Comments
m-tmatma picture m-tmatma  ·  33Comments
arigayas picture arigayas  ·  34Comments
berryzplus picture berryzplus  ·  42Comments