Subject: Re: bug#51215: Add a navigation feature to vc and diff mode: diff-goto-line

From: Juri Linkov <[email-masked]>
Date: Tue, 19 Oct 2021 09:53:44 +0300

>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=36526
>> Is a different patch than the one I committed. Mine (=Davis) does not
>> touch the vc-root-diff command it is on a strict one-file base and
>> therefore simpler.
>> Why not apply «mine» and if there is sufficient interest switch to 36526?
>
> Read further down, we've discussed (unfortunately briefly) Davis's patch
> closer to the end: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=36526#53
>
> I don't object to applying it, but like I said, it needs someone to
> champion it, do a proper review. I don't have the time or interest at the
> moment.

I have been using Davis's patch for a long time
and collected a long list of its problems below.
Later I could try to help fixing all these issues:

1. diff-goto-line doesn't work with 'C-x v D'

2. 'C-c C-d' (log-edit-show-diff) leaves point at end, should leave at beginning

3. It should be customizable

4. This feature is unusable with 'C-c C-d' (log-edit-show-diff):

5. Often when committing a change 'C-c C-d' (log-edit-show-diff)
   displays only the end of the diff buffer, so some parts of the diff
   might be committed unnoticed.  This is because 'diff-goto-line'
   moves point to some random place in the diff output.  It's difficult
   to understand its logic, why it decides to move to the middle of the
   multi-file diff.
   Sometimes displaying the end of the diff buffer makes an impression
   that it's a complete diff buffer, thus a danger to miss the
   beginning of the partially hidden diff buffer.

6. 'C-c C-d' (log-edit-show-diff)
   read-file-name("Use file /dev/null: " "/dev/" "/dev/null" t "null")
   diff-find-file-name()
   diff-goto-line("Makefile" 33 0)

7. on the last hunk it signals the error: "user-error: No next hunk"

8. compilation warnings:
   vc/diff-mode.el:706:1: Warning: Unused lexical variable `i'
   vc/diff-mode.el:733:30: Warning: reference to free variable

9. signals an error:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  string-to-number(nil)
  (let ((start (string-to-number (match-string 3))) (len (string-to-number (match-string 4))))
  (cond ((eobp) nil) ((looking-at diff-hunk-header-re-unified
  (progn (condition-case nil (diff-hunk-next)
  (while (progn (condition-case nil (diff-hunk-next)
  (if (eobp) (goto-char (point-min)) (forward-line -1) (while
  diff-goto-line("manifest.js" 1 0)
  apply(diff-goto-line ("manifest.js" 1 0))
  vc-diff-finish