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

インクリメンタルサーチ中にすべてのマッチをハイライトしたい #1104

Closed
haya14busa opened this Issue Oct 7, 2017 · 22 comments

Comments

Projects
None yet
6 participants
@haya14busa
Member

haya14busa commented Oct 7, 2017

質問・報告の内容

表題の通り、インクリメンタルサーチ中にすべてのマッチをハイライトをする機能が欲しいのでパッチを書いてみました。
vim-dev に投げる前にもしよければレビューとか意見いただけると嬉しいです。

https://github.com/haya14busa/incsearch.vim という同等の機能を提供する拙作 Vim pluginがあり、
実は個人的にはこれで全く困ってないし気に入ってるんですが、このプラグインはVimのコマンドラインをVim scriptで模倣するという"パワー"で実装されていることもあり、いくつか対応できてない機能があったり、単に hacky すぎて気に入らないユーザがたくさんいるかと思います。
 しかしすべてのマッチをハイライトする機能自体はとてもベンリなので、ぜひVim でデフォルト対応できたらなぁと思い作りました。

パッチ: https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5

helpはまだ書いてません。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 7, 2017

Member

GitHubのブランチ: https://github.com/haya14busa/vim/tree/incsearch-hi-all

なおこのパッチは @itchyny さんが vim-dev に投稿してたもの1 をベースにして、キャンセリング対応ができてなかった部分を修正したものになります。 cc/ @itchyny

Member

haya14busa commented Oct 7, 2017

GitHubのブランチ: https://github.com/haya14busa/vim/tree/incsearch-hi-all

なおこのパッチは @itchyny さんが vim-dev に投稿してたもの1 をベースにして、キャンセリング対応ができてなかった部分を修正したものになります。 cc/ @itchyny

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 7, 2017

Member

その vim-dev のスレッド1 では Bram さんは IncSearch と Search のハイライトの区別が付けば新しいオプションは必要ないという風なことをいってるかと思いますが、IncSearch/Searchのハイライトに違いが少ないcolorschemeは多々あり、また . とか検索して全部がハイライトされたときに文字自体が見づらくなるカラースキームなどもあるので、オプション足すのが丸いかなぁとおもって 'inchlsearch' という名前でオプションを作っています。 ("inc"search 中にインクリメンタルに 'hlsearch' と同様の方法でハイライトを行う)

Member

haya14busa commented Oct 7, 2017

その vim-dev のスレッド1 では Bram さんは IncSearch と Search のハイライトの区別が付けば新しいオプションは必要ないという風なことをいってるかと思いますが、IncSearch/Searchのハイライトに違いが少ないcolorschemeは多々あり、また . とか検索して全部がハイライトされたときに文字自体が見づらくなるカラースキームなどもあるので、オプション足すのが丸いかなぁとおもって 'inchlsearch' という名前でオプションを作っています。 ("inc"search 中にインクリメンタルに 'hlsearch' と同様の方法でハイライトを行う)

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 8, 2017

Member

help もやっつけですが書いてみた。

diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index 56697d51d..b52f16bf3 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -4175,7 +4175,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	'highlight' option.  This uses the "Search" highlight group by
 	default.  Note that only the matching text is highlighted, any offsets
 	are not applied.
-	See also: 'incsearch' and |:match|.
+	See also: 'incsearch', 'inchlsearch' and |:match|.
 	When you get bored looking at the highlighted matches, you can turn it
 	off with |:nohlsearch|.  This does not change the option value, as
 	soon as you use a search command, the highlighting comes back.
@@ -4386,6 +4386,17 @@ A jump table for the options with a short description can be found at |Q_op|.
 	default now.  This should work fine for most people, however if you
 	have any problem with it, try using on-the-spot style.
 
+			*'inchlsearch'* *'ihls'* *'noinchlsearch'* *'noihls'*
+'inchlsearch' 'ihls'	boolean	(default off)
+			global
+			{not in Vi}
+			{not available when compiled without the
+			|+extra_search| features}
+	While typing a search command, all matched strings are highlighted.
+	It works only when both 'incsearch' and 'hlsearch' are on. The type of
+	highlighting is same as highlight of 'hlsearch' and same as highlight
+	of 'incsearch' for the current match.
+
 						*'include'* *'inc'*
 'include' 'inc'		string	(default "^\s*#\s*include")
 			global or local to buffer |global-local|
@@ -4447,7 +4458,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	match may not be found.  This is to avoid that Vim hangs while you
 	are typing the pattern.
 	The highlighting can be set with the 'i' flag in 'highlight'.
-	See also: 'hlsearch'.
+	See also: 'hlsearch' and 'inchlsearch'.
 	CTRL-L can be used to add one character from after the current match
 	to the command line.  If 'ignorecase' and 'smartcase' are set and the
 	command line has no uppercase characters, the added character is
Member

haya14busa commented Oct 8, 2017

help もやっつけですが書いてみた。

diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index 56697d51d..b52f16bf3 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -4175,7 +4175,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	'highlight' option.  This uses the "Search" highlight group by
 	default.  Note that only the matching text is highlighted, any offsets
 	are not applied.
-	See also: 'incsearch' and |:match|.
+	See also: 'incsearch', 'inchlsearch' and |:match|.
 	When you get bored looking at the highlighted matches, you can turn it
 	off with |:nohlsearch|.  This does not change the option value, as
 	soon as you use a search command, the highlighting comes back.
@@ -4386,6 +4386,17 @@ A jump table for the options with a short description can be found at |Q_op|.
 	default now.  This should work fine for most people, however if you
 	have any problem with it, try using on-the-spot style.
 
+			*'inchlsearch'* *'ihls'* *'noinchlsearch'* *'noihls'*
+'inchlsearch' 'ihls'	boolean	(default off)
+			global
+			{not in Vi}
+			{not available when compiled without the
+			|+extra_search| features}
+	While typing a search command, all matched strings are highlighted.
+	It works only when both 'incsearch' and 'hlsearch' are on. The type of
+	highlighting is same as highlight of 'hlsearch' and same as highlight
+	of 'incsearch' for the current match.
+
 						*'include'* *'inc'*
 'include' 'inc'		string	(default "^\s*#\s*include")
 			global or local to buffer |global-local|
@@ -4447,7 +4458,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 	match may not be found.  This is to avoid that Vim hangs while you
 	are typing the pattern.
 	The highlighting can be set with the 'i' flag in 'highlight'.
-	See also: 'hlsearch'.
+	See also: 'hlsearch' and 'inchlsearch'.
 	CTRL-L can be used to add one character from after the current match
 	to the command line.  If 'ignorecase' and 'smartcase' are set and the
 	command line has no uppercase characters, the added character is
@k-takata

This comment has been minimized.

Show comment
Hide comment
@k-takata

k-takata Oct 8, 2017

Member

まだ試していませんが、意外と少ない変更で対応できるんですね。

とりあえずコーディングスタイルに関する指摘をいくつか。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L51
空行を削除していますが、残したままの方が良いかと。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L63
if{ は次の行に移動してください。ただし、{} の中身が1行の時は {} は付けないでください。(常に {} を付けるようにした方が安全でいいと思うのですが、そういうスタイルですので…。)
他の箇所も同様。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L97
変数宣言はブロックの先頭に移動してください。VCでコンパイルが通りません。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L187
ここの #ifdef の間のスペースは不要だと思います。#if 等がネストしている場合には、その階層分だけスペースを入れます。その下の数カ所も同様。
(参考にした restore_search_patterns() のインデントが間違っているようですね。一緒に直してしまってもよいと思います。)

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L213
余計なファイルが混入?

ヘルプについては、オプションを足したときは、quickref.txt 内のオプションリストも更新が必要です。
あとは、runtime/optwin.vim も更新しておいた方が本当はいいのですが、まあ、Bramに任せてもいいでしょう。

Member

k-takata commented Oct 8, 2017

まだ試していませんが、意外と少ない変更で対応できるんですね。

とりあえずコーディングスタイルに関する指摘をいくつか。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L51
空行を削除していますが、残したままの方が良いかと。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L63
if{ は次の行に移動してください。ただし、{} の中身が1行の時は {} は付けないでください。(常に {} を付けるようにした方が安全でいいと思うのですが、そういうスタイルですので…。)
他の箇所も同様。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L97
変数宣言はブロックの先頭に移動してください。VCでコンパイルが通りません。

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L187
ここの #ifdef の間のスペースは不要だと思います。#if 等がネストしている場合には、その階層分だけスペースを入れます。その下の数カ所も同様。
(参考にした restore_search_patterns() のインデントが間違っているようですね。一緒に直してしまってもよいと思います。)

https://gist.github.com/haya14busa/8c1f33aa9bf16188bc4442a8b78d30a5/9ff68be963364039bf8cca1900734e14352672da#file-inchlsearch-patch-L213
余計なファイルが混入?

ヘルプについては、オプションを足したときは、quickref.txt 内のオプションリストも更新が必要です。
あとは、runtime/optwin.vim も更新しておいた方が本当はいいのですが、まあ、Bramに任せてもいいでしょう。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 8, 2017

Member

ありがとうございます!!!直しました。ifdef そういう規約だったんですね。知らなかった...

自分の vim のリポジトリにp-r haya14busa/vim#5 して CI チェックしてみています

Member

haya14busa commented Oct 8, 2017

ありがとうございます!!!直しました。ifdef そういう規約だったんですね。知らなかった...

自分の vim のリポジトリにp-r haya14busa/vim#5 して CI チェックしてみています

@k-takata

This comment has been minimized.

Show comment
Hide comment
@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 8, 2017

Member

oh, 見落としてました 🙇 ありがとうございます。 Fixed

Member

haya14busa commented Oct 8, 2017

oh, 見落としてました 🙇 ありがとうございます。 Fixed

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Oct 8, 2017

Member

pull-req 作成時にアニメーション gif 入れていきましょう。

Member

mattn commented Oct 8, 2017

pull-req 作成時にアニメーション gif 入れていきましょう。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 9, 2017

Member

GIF撮りました 💪 🐦
peek 2017-10-09 11-01

Member

haya14busa commented Oct 9, 2017

GIF撮りました 💪 🐦
peek 2017-10-09 11-01

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 9, 2017

Member

投げました vim/vim#2198

Member

haya14busa commented Oct 9, 2017

投げました vim/vim#2198

@h-east

This comment has been minimized.

Show comment
Hide comment
@h-east

h-east Oct 10, 2017

Member

@haya14busa 👍 。ご存知だと思いますが、highlightのtestはscreenattr()を使えば可能っちゃぁ可能です。
参考になるのはどれだろう。testdir/test_match.vim とか test_matchadd_conceal_utf8.vim かなぁ。

Member

h-east commented Oct 10, 2017

@haya14busa 👍 。ご存知だと思いますが、highlightのtestはscreenattr()を使えば可能っちゃぁ可能です。
参考になるのはどれだろう。testdir/test_match.vim とか test_matchadd_conceal_utf8.vim かなぁ。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 10, 2017

Member

普通のハイライトのテストはできますがインクリメンタルサーチ中のハイライトをテストって出来ますかね……?

あぁーマッピング作ってそこから呼べばもしや可能…?

Member

haya14busa commented Oct 10, 2017

普通のハイライトのテストはできますがインクリメンタルサーチ中のハイライトをテストって出来ますかね……?

あぁーマッピング作ってそこから呼べばもしや可能…?

@h-east

This comment has been minimized.

Show comment
Hide comment
@h-east

h-east Oct 11, 2017

Member
function! GetScreenAttr(row, col)
    let g:Check_RowColAttr = [a:row, a:col, screenattr(a:row, a:col)]
    return ''
endfunction

こんな感じの関数を定義しておいて

/set<C-R>=GetScreenAttr(2,1)<CR>

とかして、後でg:Check_RowColAttrを確認すればイケそう

Member

h-east commented Oct 11, 2017

function! GetScreenAttr(row, col)
    let g:Check_RowColAttr = [a:row, a:col, screenattr(a:row, a:col)]
    return ''
endfunction

こんな感じの関数を定義しておいて

/set<C-R>=GetScreenAttr(2,1)<CR>

とかして、後でg:Check_RowColAttrを確認すればイケそう

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 11, 2017

Member

んーどうやら今回のパッチだとやはりテストムズカシそう

  1. 1文字打つ
  2. last regex pattern を保存
  3. 検索 -> last regex pattern が入力中のテキストに
  4. update_screen -> 3 で一時的にセットされたregexでハイライト
  5. 2 で保存していた regex を restore

していて、テストなどVim script から呼んだとき 4 の update_screen で redraw, スクリーンが再描画するとは限らない。

つまり <C-r>= などで関数を呼んだときにハイライトがスクリーンに反映してない。
では、その呼んだ関数で redraw すればいいという話になるけど、
5で保存していた regex にすでにrestoreされているので、ハイライトされるパターンは入力中の文字列じゃなくて、最後の検索パターン. => テストできない...

なにかいいアイデアありますかねぇ

Member

haya14busa commented Oct 11, 2017

んーどうやら今回のパッチだとやはりテストムズカシそう

  1. 1文字打つ
  2. last regex pattern を保存
  3. 検索 -> last regex pattern が入力中のテキストに
  4. update_screen -> 3 で一時的にセットされたregexでハイライト
  5. 2 で保存していた regex を restore

していて、テストなどVim script から呼んだとき 4 の update_screen で redraw, スクリーンが再描画するとは限らない。

つまり <C-r>= などで関数を呼んだときにハイライトがスクリーンに反映してない。
では、その呼んだ関数で redraw すればいいという話になるけど、
5で保存していた regex にすでにrestoreされているので、ハイライトされるパターンは入力中の文字列じゃなくて、最後の検索パターン. => テストできない...

なにかいいアイデアありますかねぇ

@ichizok

This comment has been minimized.

Show comment
Hide comment
@ichizok

ichizok Oct 11, 2017

Member

term_start() で Vim を動かして確認できないでしょうか?(GUI はテストできませんが)
test_popup.vim の Test_popup_and_window_resize() はその方法でやりました。

Member

ichizok commented Oct 11, 2017

term_start() で Vim を動かして確認できないでしょうか?(GUI はテストできませんが)
test_popup.vim の Test_popup_and_window_resize() はその方法でやりました。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 11, 2017

Member

それはなかなか(変態的で)面白いアイデアですね...!
Vim 内で Vim 起動してテストできると他のところでも使えるところありそう.

Member

haya14busa commented Oct 11, 2017

それはなかなか(変態的で)面白いアイデアですね...!
Vim 内で Vim 起動してテストできると他のところでも使えるところありそう.

@h-east

This comment has been minimized.

Show comment
Hide comment
@h-east

h-east Oct 11, 2017

Member

@haya14busa last regex patternがよく分からないです。普通に /setと入力した時のhighlight状態を数カ所\<C-R>=GetScreenAttr(~)で記録しておいて後で確認すればいいと思うのですが、私何か勘違いしてますか?
test時はredraw必要なのは理解してます。

Member

h-east commented Oct 11, 2017

@haya14busa last regex patternがよく分からないです。普通に /setと入力した時のhighlight状態を数カ所\<C-R>=GetScreenAttr(~)で記録しておいて後で確認すればいいと思うのですが、私何か勘違いしてますか?
test時はredraw必要なのは理解してます。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 11, 2017

Member

今回のパッチの実装ではいわゆる(?) @/ を一時的に上書きして hlsearch させることによってすべてのマッチをハイライトしています。この上書き&リストアは一文字入力するごとに行われるので /sett を打って次の文字を打つ時点での @/ の内容は set ではなく以前の検索パターンになっています。
なので関数内でredrawしても以前の検索パターンの状態でハイライトされてしまうということです。

Member

haya14busa commented Oct 11, 2017

今回のパッチの実装ではいわゆる(?) @/ を一時的に上書きして hlsearch させることによってすべてのマッチをハイライトしています。この上書き&リストアは一文字入力するごとに行われるので /sett を打って次の文字を打つ時点での @/ の内容は set ではなく以前の検索パターンになっています。
なので関数内でredrawしても以前の検索パターンの状態でハイライトされてしまうということです。

@h-east

This comment has been minimized.

Show comment
Hide comment
@h-east

h-east Oct 11, 2017

Member

なるほど。う~む。。。新たにhighlightグループ作るのも煩雑になるしなぁ。。

Member

h-east commented Oct 11, 2017

なるほど。う~む。。。新たにhighlightグループ作るのも煩雑になるしなぁ。。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 14, 2017

Member

term_start() でテスト書けました...!
vim/vim@a552edd

ちなみに IncSeach のハイライトもredrawでテストしてたときには消えてたっぽいので別のハイライトグループというのもそうですが、そもそもハイライトの仕方を考えないとダメそうかも...?(この辺は勘で言っています)

Member

haya14busa commented Oct 14, 2017

term_start() でテスト書けました...!
vim/vim@a552edd

ちなみに IncSeach のハイライトもredrawでテストしてたときには消えてたっぽいので別のハイライトグループというのもそうですが、そもそもハイライトの仕方を考えないとダメそうかも...?(この辺は勘で言っています)

@tyru

This comment has been minimized.

Show comment
Hide comment
@tyru

tyru Oct 29, 2017

Member

おめでとうございます! @haya14busa ++ >8.0.1238

Member

tyru commented Oct 29, 2017

おめでとうございます! @haya14busa ++ >8.0.1238

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Oct 29, 2017

Member

やりましたー! 💪 🐦

みなさまご協力ありがとうございましたっ!!

Member

haya14busa commented Oct 29, 2017

やりましたー! 💪 🐦

みなさまご協力ありがとうございましたっ!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment