GNU bug report logs - #62731
29.0.60; diff-apply-hunk doesn't work for creating new files

Previous Next

Full log

Message #40 received at 62731 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: sbaugh <at> catern.com, 62731 <at> debbugs.gnu.org
Subject: Re: bug#62731: 29.0.60; diff-apply-hunk doesn't work for creating
 new files
Date: Thu, 17 Oct 2024 20:56:38 +0300

>>>> All right, the attached seems to support both creation and deletion,
>>>> including applying hunks in reverse direction.
>>>> Things got trickier but not by a lot.
>>>
>>> Now pushed to master, seems useful enough. Let's see if some unforeseen
>>> problems are reported.
>> 
>> This change broke diff of files:
>> 
>> @@ -1957,7 +1970,7 @@ diff-find-source-location
>>                                   diff-context-mid-hunk-header-re nil t)
>>   			 (error "Can't find the hunk separator"))
>>   		       (match-string 1)))))
>> -	   (file (or (diff-find-file-name other noprompt)
>> +	   (file (or (diff-find-file-name (xor other reverse) noprompt)
>>                        (error "Can't find the file")))
>>   	   (revision (and other diff-vc-backend
>>                             (if reverse (nth 1 diff-vc-revisions)
>> 
>> So after 'dired-backup-diff', typing 'C-c C-c' visits wrong file:
>> visits the backup when point is on the file line, and vice versa:
>> visits the file when point is on backup file line.
>
> Thanks for reporting.
>
> Do you think we can fix it this way?
>
> diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
> index cfa90d380ad..374df3ee2cb 100644
> --- a/lisp/vc/diff-mode.el
> +++ b/lisp/vc/diff-mode.el
> @@ -2196,7 +2196,7 @@ diff-goto-source
>     ;; This is a convenient detail when using smerge-diff.
>     (if event (posn-set-point (event-end event)))
>     (let ((buffer (when event (current-buffer)))
> -        (reverse (not (save-excursion (beginning-of-line) (looking-at "[-<]")))))
> +        (reverse (not (save-excursion (beginning-of-line) (looking-at "[+<]")))))
>       (pcase-let ((`(,buf ,_line-offset ,pos ,src ,_dst ,_switched)
>                    (diff-find-source-location other-file reverse)))
>         (pop-to-buffer buf)

I don't understand this change.

> Otherwise, I don't quite understand the intent behind that line. But it 
> might have been masking the problem which I (hopefully) fixed in the 
> hunk you quoted/

There is already one 'xor' here.  Maybe the second 'xor' not needed:

(defun diff-find-source-location (&optional other-file reverse noprompt)
    ...
    (let* ((other (xor other-file diff-jump-to-old-file))
    ...
	   (file (or (diff-find-file-name (xor other reverse) noprompt)

Previous Next

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