Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation

From: [email-masked]
Date: Mon, 14 Nov 2016 22:31:17 -0500

Dima Kogan <[email-masked]> writes:

> [email-masked] writes:
>
>> Dima Kogan <[email-masked]> writes:
>>
>>> Hi. I'm open to suggestions. The goal was to retain the previous logic
>>> for any existing code, but to provide improved user-facing behavior.
>
> Ack! So this is what happens when you submit a patch, and then talk
> about it more than 2 years later. Turns out preserving compatibility
> with existing code wasn't my goal at all. Rather, the goal was to avoid
> an infinite loop that results if (diff-hunk-next) unconditionally uses
> the new logic:
>
>     diff-hunk-next         calls
>     diff--wrap-navigation  calls
>     diff-bounds-of-hunk    calls
>     diff-beginning-of-hunk calls
>     diff-hunk-next
>
> Revised patch attached. I'll try to respond to any comments
> quickly so that we all can remember what this is about.
[...]
> -	    (t (error "No hunk found"))))))
> +            ;; There's no next hunk, so just take the one we have
> +	    (t (list beg end))))))

Indentation on the comment looks a bit off.

> +
> +      ;; I advance to the next hunk interactively because I want the
> +      ;; interactive behavior of moving to the next logical hunk, not
> +      ;; the legacy behavior where were would sometimes sty on the
> +      ;; curent hunk.  See http://debbugs.gnu.org/17544
>        (when diff-advance-after-apply-hunk
> -	(diff-hunk-next))))))
> +        (diff-hunk-next nil t))))))

Updating the comment here will be useful for the next person trying to
figure out what this is all about in a couple more years.

>  	 (hunk (delete-and-extract-region
> -		(point) (save-excursion (diff-end-of-hunk) (point))))
> +                (point) (cadr hunk-bounds)))

Indentation looks off here.