GNU bug report logs - #17544
24.3; [PATCH] Improved diff-mode navigation/manipulation

Previous Next

Package: emacs;

Reported by: Dima Kogan <lists <at> dima.secretsauce.net>

Date: Wed, 21 May 2014 15:00:05 UTC

Severity: normal

Tags: patch

Found in version 24.3

Fixed in version 26.1

Done: npostavs <at> users.sourceforge.net

Bug is archived. No further changes may be made.

Full log


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

From: npostavs <at> users.sourceforge.net
To: Dima Kogan <dima <at> secretsauce.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Andreas Schwab <schwab <at> linux-m68k.org>,
 17544 <at> debbugs.gnu.org
Subject: Re: bug#17544: 24.3;
 [PATCH] Improved diff-mode navigation/manipulation
Date: Sat, 22 Oct 2016 22:49:21 -0400
Dima Kogan <dima <at> secretsauce.net> writes:

> npostavs <at> users.sourceforge.net writes:
>
>> Dima Kogan <dima <at> secretsauce.net> writes:
>>
>>> Here's yet another revised patch. The (call-interactively) at the end of
>>> (diff-apply-hunk) was important, it turns out. This would force it to
>>> use the new logic to move to the next hunk, instead of the legacy logic.
>>> I purposely left the behavior of (diff-next-hunk) unchanged from before
>>> when running non-interactively, and here I explicitly want the new
>>> behavior.
>>
>> If both behaviours are needed, it would be much better if lisp code
>> could choose between them without having to use call-interactively,
>> that's quite an awkward interface.
>
> 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.
> Given this, it doesn't seem to me to be too awkward to pass-on the
> "interactive-p" state to child functions.
>

But you're synthesizing interactiveness in diff-apply-hunk, so it looks
like interactiveness is not what you actually care about.  You could do
something like this instead:

(defun diff-hunk-next (&optional count skip-hunk-start)
  "Go to the next COUNT'th hunk"
  (interactive (list (prefix-numeric-value current-prefix-arg) t))
  (diff--wrap-navigation
   skip-hunk-start
   "next hunk"
   'diff--internal-hunk-next
   diff-hunk-header-re
   (lambda () (goto-char (car (diff-bounds-of-hunk))))
   count))

Then instead of (call-interactively 'diff-hunk-next) you can just do
(diff-hunk-next nil t)

> Am I wrong to want to preserve
> existing behavior for elisp code? If so, then the entire old path can
> simply go away unconditionally.

Maybe.  What about the other calls to diff-hunk-next?  Was there a
reason you kept them the same?




This bug report was last modified 8 years and 180 days ago.

Previous Next


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