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

From: Dmitry Gutov <[email-masked]>
Date: Wed, 20 Oct 2021 02:18:07 +0300

On 19.10.2021 09:53, Juri Linkov wrote:
>>> 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:

Thank you for testing and cataloguing the problems. So it needs more 
work. I'll Cc Davis, perhaps he would like to continue the effort 
himself, too. Or to at least be aware of the progress.

> 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
>