vc-buffer-sync: Wrong type argument: stringp, nil in indirect buffers

[mookid]

Steps:

• run emacs -Q -l path/to/diff-hl.el file-under-version-control
• M-x clone-indirect-buffer
• In the new buffer, run diff-hl-diff-goto-hunk. One get the error vc-buffer-sync: Wrong type argument: stringp, nil with the following backtrace:

  file-exists-p(nil)
  vc-buffer-sync()
  diff-hl-diff-goto-hunk()
  funcall-interactively(diff-hl-diff-goto-hunk)
  call-interactively(diff-hl-diff-goto-hunk record nil)

• In the same buffer, vc-diff fails more gracefully with vc-deduce-fileset: Buffer [...] is not associated with a file

Note that the following path seems to be enough to solve the issue:

diff --git a/diff-hl.el b/diff-hl.el
index f504ddf..bd3caf6 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -379,14 +379,15 @@ the end position as its only argument."
 (defun diff-hl-diff-goto-hunk ()
   "Run VC diff command and go to the line corresponding to the current."
   (interactive)
-  (vc-buffer-sync)
-  (let* ((line (line-number-at-pos))
-         (buffer (current-buffer)))
-    (vc-diff-internal t (vc-deduce-fileset) diff-hl-reference-revision nil t)
-    (vc-exec-after `(if (< (line-number-at-pos (point-max)) 3)
-                        (with-current-buffer ,buffer (diff-hl-remove-overlays))
-                      (diff-hl-diff-skip-to ,line)
-                      (setq vc-sentinel-movepoint (point))))))
+  (with-current-buffer (or (buffer-base-buffer) (current-buffer))
+    (vc-buffer-sync)
+    (let* ((line (line-number-at-pos))
+           (buffer (current-buffer)))
+      (vc-diff-internal t (vc-deduce-fileset) diff-hl-reference-revision nil t)
+      (vc-exec-after `(if (< (line-number-at-pos (point-max)) 3)
+                          (with-current-buffer ,buffer (diff-hl-remove-overlays))
+                        (diff-hl-diff-skip-to ,line)
+                        (setq vc-sentinel-movepoint (point)))))))

edit: this is GNU Emacs 27.0.50 (build 1, x86_64-w64-mingw32) of 2019-11-11

[dgutov]

Hi!

I've never used clone-indirect-buffer interactively. What do you use it for?

[mookid]

I usually use it after mark-defun to make a view of a given buffer, in combination with narrowing. That way, I can get the benefit of narrowing (making a small view of a buffer) while still having access to the whole buffer.

[dgutov]

Would it be much of a loss to disable diff-hl in indirect buffers instead?

[mookid]

Well, I can always work around that limitation with an advice, so no big change for me. If you think that the proper fix should be more complex, no problem (but I think that the current state is poor).

[dgutov]

shrug If it really improves your experience, please go ahead and make a PR, I'll merge.

No solid promise not to remove this functionality later (I still don't know how feasible it is to support indirect buffers in the long run), but it'll probably stay.

[mookid]

@dgutov I can try to add a test for that, if it makes things easier to maintain (I see no tests currently), unless you do not want tests in the repository.

[dgutov]

If you care to add a few tests, that would only be welcome. So far I've been too lazy to do it here, and manual testing was easy enough because the package doesn't offer many options.

(You have done copyright assignment already, right?)

[mookid]

(You have done copyright assignment already, right?)

I do!