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

アウトライン解析の選択行追従 #1398

Merged
merged 5 commits into from
Sep 14, 2020

Conversation

suconbu
Copy link
Member

@suconbu suconbu commented Sep 12, 2020

PR の目的

エディタ上でのキャレット移動にあわせてアウトライン解析の選択行の位置が自動的に更新されるようにします。

動作イメージ:
outlineauto_demo png

カテゴリ

  • 機能追加

PR の背景

コードリーディング時、今いる関数を確認するために F11 キーを二連打 (フォーカス→更新) していましたが、これが煩わしいと感じていたため。

PR のメリット

キャレット移動した時、追加の操作なく選択行の位置が更新されるため便利になります。

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

アウトライン解析表示時におけるキャレット移動時の負荷が増加します。
が、通常使用時 (数万行程度までのソースコード閲覧等) においてはほぼ無視できる程度かなと考えています。

仕様・動作説明

  • キャレット移動に追従してアウトライン解析・ブックマーク一覧の選択行を更新します。
  • テキストの内容が変更された時には追従を解除し、最後の選択位置を維持します。
  • 解除された追従は再解析が実行された時に復帰します。

テスト内容

テスト用ファイル:test.zip

テスト1 - 追従の確認 (アウトライン解析)

  1. test.c を開く。
  2. キャレットを "func2" の行まで移動する。
  3. F11 キーを押下してアウトライン解析を表示する。
  • 確認1: アウトライン解析の func2 の行が選択されていること。
  1. ファイル先頭からファイル末尾まで一行ずつキャレットを移動する。
  • 確認2: アウトライン解析の選択行が移動に合わせて更新されること。

同様のテストを test.cpp, test.vb, test.txt で実施する。

テスト2 - 追従の確認 (ブックマーク一覧)

  1. test.c を開く。
  2. "func" を含む行すべてにブックマークを設定する。
  3. ブックマーク一覧を開く。
  4. ファイル先頭にキャレットを置いた状態で F2 キーを押下する。
  • 確認1: アウトライン解析の選択行が更新されること。

テスト3 - 追従解除・復帰の確認

  1. test.c を開く。
  2. func1 の "}" の行にキャレットを置いた状態で Enter キーを 3 回押下する。
  • 確認内容1: アウトライン解析の選択行が func1 の位置のまま移動しないこと。
  1. "func2" の行までキャレットを移動してアウトライン解析の更新ボタンを押下する。
  • 確認2: アウトライン解析の func2 の行数表示が更新されること。
  • 確認3: アウトライン解析の選択行が func2 へ移動すること。
  1. ファイルを上書き保存する。
  2. ファイル先頭からファイル末尾まで一行ずつキャレットを移動する。
  • 確認4: アウトライン解析の選択行が移動に合わせて更新されること。

テスト4 - 性能劣化の確認

  1. test_100k.txt を開く。
  2. キャレットをファイル末尾まで移動して F11 キーを押下する。
  3. アウトライン解析の内容が表示されるまで待つ。
  4. キャレットを上下に移動する。
  • 確認1: キャレットの移動が極端にもたつかないこと。

備考

ファイルツリーについては、今回の変更の前から常に先頭行が選択状態になってしまう?ためテスト対象から除外しました。

PR の影響範囲

アウトライン解析・ブックマーク一覧・ファイルツリー

関連 issue, PR

参考資料

@AppVeyorBot
Copy link

Build sakura 1.0.3078 completed (commit 9f94346593 by @suconbu)

@usagisita
Copy link
Contributor

かなり面白いと思います。

一点だけ気になるところがありまして、C言語だとリストビューなので気にならないのですが、ツリービューだと縮小しているツリーを上下キーなどで移動しているとどんどん展開されていって、縮小しておいた見通しのいいツリーが大変なことになります。
レベル1まで展開して、大見出し間だけ移動するのに使う、みたいなケースだと、ちょっと使いづらいかなあと思いまして、しゃしゃりでてきました。
追随のON・OFF設定をアウトラインのドッキングオプションとかのあたりに追加していただけると、ありがたいかもしれません。まあ、僕の意見は参考程度にお願いします。

@suconbu
Copy link
Member Author

suconbu commented Sep 12, 2020

興味を持っていただきありがとうございます!

ツリービューだと縮小しているツリーを上下キーなどで移動しているとどんどん展開されていって、縮小しておいた見通しのいいツリーが大変なことになります。

懸念されている状況を理解できました。
選択位置を更新する時には、現在のツリーの展開状態をキープする (勝手に展開しない) 振る舞いとすれば、追随そのものは活かしたままでも懸念点を解消できそうですね。
多分、常にこの自動展開しない動作で困ることはない気がしています。

@suconbu suconbu changed the title アウトライン解析の選択行追従 [WIP] アウトライン解析の選択行追従 Sep 12, 2020
@beru beru added the enhancement ■機能追加 label Sep 12, 2020
@suconbu suconbu force-pushed the feature/add_outline_selection_auto branch from 27829eb to 61df29a Compare September 12, 2020 13:38
@suconbu
Copy link
Member Author

suconbu commented Sep 12, 2020

追従時にツリーの展開状態をキープするようにしてみました。

outlineauto_keeptreeexp

@AppVeyorBot
Copy link

Build sakura 1.0.3080 completed (commit 37f1a89efc by @suconbu)

@AppVeyorBot
Copy link

Build sakura 1.0.3081 completed (commit a220f8b7c4 by @suconbu)

@AppVeyorBot
Copy link

Build sakura 1.0.3082 completed (commit 788d9ca9ec by @suconbu)

@suconbu suconbu force-pushed the feature/add_outline_selection_auto branch from 6377261 to cb91dbf Compare September 13, 2020 02:25
@suconbu suconbu changed the title [WIP] アウトライン解析の選択行追従 アウトライン解析の選択行追従 Sep 13, 2020
@AppVeyorBot
Copy link

Build sakura 1.0.3084 completed (commit c9325a180f by @suconbu)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

デフォルトでは行が昇順で並び替えられていて、その際の挙動は問題無いように思えます。

ListViewの列ヘッダーをクリックして行を降順に並び替えたり、関数名で並び替えると挙動が変になります。

@suconbu
Copy link
Member Author

suconbu commented Sep 13, 2020

ListViewの列ヘッダーをクリックして行を降順に並び替えたり、関数名で並び替えると挙動が変になります。

確認頂きありがとうございます。私の方でも現象を確認できました。
CDlgFuncList::SetItemSelectionFor* にて、TreeView の方が各アイテムの lParam の値で紐づけをしているのに対し、ListView は単純にインデックスで選択対象を決定しているためにおかしくなっていました。
ListView でも lParam が利用できるか確認しつつこれを見るように修正します。

@suconbu
Copy link
Member Author

suconbu commented Sep 13, 2020

ListViewの列ヘッダーをクリックして行を降順に並び替えたり、関数名で並び替えると挙動が変になります。

こちら解消することができました。ご確認お願いします。

@AppVeyorBot
Copy link

Build sakura 1.0.3092 completed (commit 70e8a44b33 by @suconbu)

@suconbu
Copy link
Member Author

suconbu commented Sep 13, 2020

動作仕様を決める中で少し迷った点があったため記載いたします。

PR 説明の「テスト3」に書きましたように、テキストが編集された時には選択位置の追従を解除しますが、
この時のアウトライン解析上の選択項目について、以下の二つが考えられました。

  1. 最後の位置のまま残す
  2. 選択を解除する

「2」は追従が解除されたことが視覚的にわかりやすい反面、今編集中の段落・関数を見失ってしまう懸念がありました。
そのため、今回は変更前の振る舞いに近い「1」の方を採用しています。

追従解除中には項目の選択色を淡色に変更する、などの改善案を思いつきはしましたが、変更箇所が多くなりすぎてしまいそうでしたので今回は現動作で PR 出させて頂きました。

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

アウトライン解析が「C 関数一覧」の時の挙動は問題無いと思います。

「テキスト トピックツリー」の時に以下の点が気になります。

  • 入力内容に応じてリアルタイムにトピックツリーの表示が更新されない
  • F11キーを2回連続で押すとトピックツリーの開閉状態が変わる(一番最後のノードまで開くがそれ以外は開かない)

ただこれに関しては元からそうなのでこのPRで挙動を変える必要も無いと思います。

という事でこのPRの目的は達成されていると思うので問題無いと思います。

仮に、ソースコードのカーソル移動時にアウトライン解析の選択状態を変えてほしく無いんだよ!という要望が後で出たら、それはまた後で考えれば良いかなと思います。

@beru
Copy link
Contributor

beru commented Sep 14, 2020

こちら自分はコードレビューはちゃんとしてなくて軽い動作確認しかしてませんが Merge します。

後で問題が見つかったら直せば良いと思います。

@beru beru merged commit 5d9f254 into sakura-editor:master Sep 14, 2020
@suconbu
Copy link
Member Author

suconbu commented Sep 15, 2020

レビュー・マージありがとうございました。

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

Successfully merging this pull request may close these issues.

4 participants