GNU bug report logs - #51215
Add a navigation feature to vc and diff mode: diff-goto-line

Previous Next

Full log

View this message in rfc822 format

From: Uwe Brauer <oub <at> mat.ucm.es>
To: 51215 <at> debbugs.gnu.org
Subject: bug#51215: Add a navigation feature to vc and diff mode: diff-goto-line
Date: Mon, 01 Nov 2021 08:24:04 +0100

[Message part 1 (text/plain, inline)]

>>> "JL" == Juri Linkov <juri <at> linkov.net> writes:

Hi Juri

I did not see your message, thanks very much for this detailed
information


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


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

Right, that is the vc-root-diff command. As I said before in my
understanding Davis patch is just suited for a one-file-analysis. So
that could later be added

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

Oh, I admit it is the first time I come across that command (or I have used it in the past and forgot it).

When I try it out I obtain 
,----
| Debugger entered--Lisp error: (error "Diff functionality has not been setup")
|   signal(error ("Diff functionality has not been setup"))
|   error("Diff functionality has not been setup")
|   #f(compiled-function () #<bytecode 0x9abeb64133bec99>)()
|   log-edit-show-diff()
|   funcall-interactively(log-edit-show-diff)
|   apply(funcall-interactively log-edit-show-diff nil)
|   repeat-complex-command(1)
|   funcall-interactively(repeat-complex-command 1)
|   call-interactively(repeat-complex-command nil nil)
|   command-execute(repeat-complex-command)
`----

The documentation of that function is sparse to say the least.
Could that be a git only thing?
That is getting a bit off topic but any advice would be appreciated.


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

I need to set this up to reproduce it

> 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

Well that was written in 2016 without lexical binding I suppose

> 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

I can reproduce this. Can you give me a recipe


Uwe 

[smime.p7s (application/pkcs7-signature, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.