set hidden 時に diff 状態が window を閉じても解除されない

[lambdalisue]

質問・報告の内容

set hidden によりバッファを隠す設定の場合ウィンドウを閉じても diffthis 状態が解除されないため diff の結果がおかしくなります。

https://asciinema.org/a/2mdgrmk21zim4ixty4iehlc9b

また、この状態では以下のコマンドでは隠れたバッファの diff 状態が解除できませんでした。

:diffoff!
:windo diffoff!
:bufdo diffoff!

そのため動画最後に c.txt に対して diffthis しているように、全く異なるファイルに対して diffthis を行った際にワケガワカラナイヨ

Vimのバージョン

• Vim 8.0.596（Vim 7.4 時代からずっと存在している問題だと思います）

( :version で確認できます)

OSの種類/ディストリ/バージョン

• macOS Sierra (この issue 記載時に確認を取った OS です)
• Ubuntu 16.04 (記載時に確認を取ったわけではなく、ずっと前から問題を認識していたって感じです)

再現手順

前準備

echo "Hello\nWorld" > a.txt
echo "Hello\nworld" > b.txt
echo "foobar" > c.txt

実行 (vim -u NONE)

:set hidden  " これが原因
:e a.txt
:vnew
:e b.txt
:diffthis
:wincmd w
:diffthis
:q

期待動作

比較対象が居なくなるため、ハイライトが消える（set nohidden 時の挙動）

実際の動作

隠れたバッファと比較し続けるため、ハイライトが消えない。

さらに、副次的な以下のような問題もあります

:e c.txt
:diffthis

期待動作

比較対象が居ないため、何も起こらない

実際の動作

隠れたバッファ a.txt および b.txt との比較をするため、ハイライトが酷いことになる（動画の最後参照）

[h-east]

最初の動画を見た感じだと、再現手順が足りていないように思います。
以下を足せば良い？

:e c.txt
:diffthis

出来れば再現手順の下に期待する動作と実際の動作を書いていただけると嬉しいです。

（set hiddenの有無で:e c.txt後の:diffthisの結果が異なるのがバグだというのは理解できました）

[h-east]

あっ、バグといっちゃいましたが、「set hiddenの時に隠れ状態になったバッファのdiff関連のオプションがリセットされないのが正しいのか正しくないのか」はドキュメントやソースコードを見て判断したいと思います。

[lambdalisue]

あー裏側に残ってしまうことの再現手順を書いてました。裏側に残ったものと比較してしまうってのは副次的なものと考えていたためです。言葉足らずでしたすみません。

ドキュメントを見ると diffthis はウインドウに対してのオプションですが、その下の方に8つのバッファまで適用可能と書いてあり正しい挙動がわからない感じです‥‥。実用的には開いていないバッファと比較しても仕方ない気もするので issue として報告させていただきました。

[h-east]

@lambdalisue Issue本文の修正ありがとうございます。再現手順足りてました。私の勘違いでした。

このpatchで @lambdalisue さんの意図する挙動になるかと思います。確認をお願いします。

diff --git a/src/buffer.c b/src/buffer.c
index 83d74a3..dfdb97e 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -572,6 +572,11 @@ aucmd_abort:
     if (buf->b_nwindows > 0)
 	--buf->b_nwindows;
 
+#ifdef FEAT_DIFF
+    if (!unload_buf && buf->b_nwindows == 0)
+	diff_buf_delete(buf);	/* Clear 'diff' for hidden buffer. */
+#endif
+
     /* Return when a window is displaying the buffer or when it's not
      * unloaded. */
     if (buf->b_nwindows > 0 || !unload_buf)

ただ、このpatchをあてた状態で make test するとtestがコケます。

From test_diffmode.vim:
Found errors in Test_diffoff_hidden():
function RunTheTest[24]..Test_diffoff_hidden line 16: Expected not equal to 0
function RunTheTest[24]..Test_diffoff_hidden line 23: Expected not equal to 0
TEST FAILURE

diffmodeとhiddenの組み合わせのtestが存在していますねー
vim_devにpatchを提案する場合は、上記testを修正する必要があります。
あと、関連する部分で仕様を壊していないか検討が必要です。

[lambdalisue]

ありがとうございます！週末に確認して見ます:+1:

[mattn]

ping

[lambdalisue]

うお、完全に忘れてました‥‥明日確認します。

[lambdalisue]

試してみました。完全に想定通りの挙動です 👍

テストが失敗するということで diffopt に hiddenoff （名前は適当）がある場合のみ、この挙動になるようにしてみました。どうでしょう？ (Vim 8.0.0691 - a83fe75ca7)

diff --git a/src/buffer.c b/src/buffer.c
index e8e1a6de0..7ad2b370b 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -577,6 +577,11 @@ aucmd_abort:
     if (buf->b_nwindows > 0)
 	--buf->b_nwindows;
 
+#ifdef FEAT_DIFF
+    if (diffopt_hiddenoff() && !unload_buf && buf->b_nwindows == 0)
+	diff_buf_delete(buf);	/* Clear 'diff' for hidden buffer. */
+#endif
+
     /* Return when a window is displaying the buffer or when it's not
      * unloaded. */
     if (buf->b_nwindows > 0 || !unload_buf)
diff --git a/src/diff.c b/src/diff.c
index dfc968f83..330b88ffc 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -23,6 +23,7 @@ static int	diff_busy = FALSE;	/* ex_diffgetput() is busy */
 #define DIFF_IWHITE	4	/* ignore change in white space */
 #define DIFF_HORIZONTAL	8	/* horizontal splits */
 #define DIFF_VERTICAL	16	/* vertical splits */
+#define DIFF_HIDDEN_OFF	32	/* diffoff when hidden */
 static int	diff_flags = DIFF_FILLER;
 
 #define LBUFLEN 50		/* length of line in diff file */
@@ -1898,6 +1899,11 @@ diffopt_changed(void)
 	    p += 11;
 	    diff_foldcolumn_new = getdigits(&p);
 	}
+	else if (STRNCMP(p, "hiddenoff", 9) == 0)
+	{
+	    p += 9;
+	    diff_flags_new |= DIFF_HIDDEN_OFF;
+	}
 	if (*p != ',' && *p != NUL)
 	    return FAIL;
 	if (*p == ',')
@@ -1936,6 +1942,15 @@ diffopt_horizontal(void)
 }
 
 /*
+ * Return TRUE if 'diffopt' contains "hiddenoff".
+ */
+    int
+diffopt_hiddenoff(void)
+{
+    return (diff_flags & DIFF_HIDDEN_OFF) != 0;
+}
+
+/*
  * Find the difference within a changed line.
  * Returns TRUE if the line was added, no other buffer has it.
  */
diff --git a/src/proto/diff.pro b/src/proto/diff.pro
index 718640152..f1afeecec 100644
--- a/src/proto/diff.pro
+++ b/src/proto/diff.pro
@@ -16,6 +16,7 @@ int diff_check_fill(win_T *wp, linenr_T lnum);
 void diff_set_topline(win_T *fromwin, win_T *towin);
 int diffopt_changed(void);
 int diffopt_horizontal(void);
+int diffopt_hiddenoff(void);
 int diff_find_change(win_T *wp, linenr_T lnum, int *startp, int *endp);
 int diff_infold(win_T *wp, linenr_T lnum);
 void nv_diffgetput(int put, long count);

[h-east]

diffopt に hiddenoff （名前は適当）がある場合のみ、この挙動になるようにしてみました。どうでしょう？

おぉ、良いと思います:+1:
ドキュメントとテストを追加してvim_devに提案すれば受け入れられそう。
@lambdalisue さん、ドキュメントとテストの追加できそうならお願いします。

[lambdalisue]

わかりました、また動けるのは来週末以降になると思いますがやってみますー 👍

[lambdalisue]

とんでもなく遅くなりましたがテスト追加とドキュメント追加を行ってみました。
ご確認お願いします。

Vim: f38c86eb6 patch 8.0.1352: dead URLs in the help go unnoticed

diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index c4ceb5f39..818755c10 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -2634,6 +2634,9 @@ A jump table for the options with a short description can be found at |Q_op|.
 		vertical	Start diff mode with vertical splits (unless
 				explicitly specified otherwise).
 
+		hiddenoff	Automatically diffoff when a buffer has been
+				hidden.
+
 		foldcolumn:{n}	Set the 'foldcolumn' option to {n} when
 				starting diff mode.  Without this 2 is used.
 
diff --git a/src/buffer.c b/src/buffer.c
index dad910388..ed5773e88 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -593,6 +593,11 @@ aucmd_abort:
     if (buf->b_nwindows > 0)
 	--buf->b_nwindows;
 
+#ifdef FEAT_DIFF
+    if (diffopt_hiddenoff() && !unload_buf && buf->b_nwindows == 0)
+    	diff_buf_delete(buf);	/* Clear 'diff' for hidden buffer. */
+#endif
+
     /* Return when a window is displaying the buffer or when it's not
      * unloaded. */
     if (buf->b_nwindows > 0 || !unload_buf)
diff --git a/src/diff.c b/src/diff.c
index 11952c743..2f5b38c7b 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -23,6 +23,7 @@ static int	diff_busy = FALSE;	/* ex_diffgetput() is busy */
 #define DIFF_IWHITE	4	/* ignore change in white space */
 #define DIFF_HORIZONTAL	8	/* horizontal splits */
 #define DIFF_VERTICAL	16	/* vertical splits */
+#define DIFF_HIDDEN_OFF	32	/* diffoff when hidden */
 static int	diff_flags = DIFF_FILLER;
 
 #define LBUFLEN 50		/* length of line in diff file */
@@ -1924,6 +1925,11 @@ diffopt_changed(void)
 	    p += 11;
 	    diff_foldcolumn_new = getdigits(&p);
 	}
+	else if (STRNCMP(p, "hiddenoff", 9) == 0)
+	{
+	    p += 9;
+	    diff_flags_new |= DIFF_HIDDEN_OFF;
+	}
 	if (*p != ',' && *p != NUL)
 	    return FAIL;
 	if (*p == ',')
@@ -1962,6 +1968,15 @@ diffopt_horizontal(void)
 }
 
 /*
+ * Return TRUE if 'diffopt' contains "hiddenoff".
+ */
+    int
+diffopt_hiddenoff(void)
+{
+    return (diff_flags & DIFF_HIDDEN_OFF) != 0;
+}
+
+/*
  * Find the difference within a changed line.
  * Returns TRUE if the line was added, no other buffer has it.
  */
diff --git a/src/proto/diff.pro b/src/proto/diff.pro
index 718640152..f1afeecec 100644
--- a/src/proto/diff.pro
+++ b/src/proto/diff.pro
@@ -16,6 +16,7 @@ int diff_check_fill(win_T *wp, linenr_T lnum);
 void diff_set_topline(win_T *fromwin, win_T *towin);
 int diffopt_changed(void);
 int diffopt_horizontal(void);
+int diffopt_hiddenoff(void);
 int diff_find_change(win_T *wp, linenr_T lnum, int *startp, int *endp);
 int diff_infold(win_T *wp, linenr_T lnum);
 void nv_diffgetput(int put, long count);
diff --git a/src/testdir/test_diffmode.vim b/src/testdir/test_diffmode.vim
index 3273dcbd5..d6c5c83fe 100644
--- a/src/testdir/test_diffmode.vim
+++ b/src/testdir/test_diffmode.vim
@@ -375,6 +375,29 @@ func Test_diffopt_vertical()
   %bwipe
 endfunc
 
+func Test_diffopt_hiddenoff()
+  set diffopt=filler,foldcolumn:0,hiddenoff
+  e! one
+  call setline(1, ['Two', 'Three'])
+  redraw
+  let normattr = screenattr(1, 1)
+  diffthis
+  botright vert new two
+  call setline(1, ['One', 'Four'])
+  diffthis
+  redraw
+  call assert_notequal(normattr, screenattr(1, 1))
+  set hidden
+  close
+  redraw
+  " should not diffing with hidden buffer two while 'hiddenoff' is enabled
+  call assert_equal(normattr, screenattr(1, 1))
+
+  bwipe!
+  bwipe!
+  set hidden& diffopt&
+endfunc
+
 func Test_diffoff_hidden()
   set diffopt=filler,foldcolumn:0
   e! one

[h-east]

@lambdalisue patchの内容を確認しました。問題ないと思います:+1:

[lambdalisue]

ありがとうございます。 この後の流れは 1. 上記内容で GitHub の PR 2. vim-dev のメーリングリスト（購読すらしていないからよくわかりませんが‥‥） のどちらが良いでしょうか？あと何か気をつけることありますか？初回なので色々分からず、すみません。

…

On Dec 1, 2017 02:32 +0900, h_east ***@***.***>, wrote: @lambdalisue patchの内容を確認しました。問題ないと思います👍 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.