From debbugs-submit-bounces@debbugs.gnu.org Wed May 21 10:59:56 2014 Received: (at submit) by debbugs.gnu.org; 21 May 2014 14:59:56 +0000 Received: from localhost ([127.0.0.1]:55447 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Wn7zm-00056c-UH for submit@debbugs.gnu.org; Wed, 21 May 2014 10:59:56 -0400 Received: from eggs.gnu.org ([208.118.235.92]:43869) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Wn5Wx-0008Uz-1y for submit@debbugs.gnu.org; Wed, 21 May 2014 08:21:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn5Wl-0001G2-7Z for submit@debbugs.gnu.org; Wed, 21 May 2014 08:21:53 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,T_DKIM_INVALID autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:52388) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn5Wl-0001Fy-4e for submit@debbugs.gnu.org; Wed, 21 May 2014 08:21:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45563) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn5Wf-0002i1-6H for bug-gnu-emacs@gnu.org; Wed, 21 May 2014 08:21:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn5WZ-00013Y-CV for bug-gnu-emacs@gnu.org; Wed, 21 May 2014 08:21:41 -0400 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:48905) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn5WZ-000138-70 for bug-gnu-emacs@gnu.org; Wed, 21 May 2014 08:21:35 -0400 Received: from compute4.internal (compute4.nyi.mail.srv.osa [10.202.2.44]) by gateway1.nyi.mail.srv.osa (Postfix) with ESMTP id DAA90210F3 for ; Wed, 21 May 2014 08:21:31 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute4.internal (MEProxy); Wed, 21 May 2014 08:21:32 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=from:to:subject:date:message-id :mime-version:content-type; s=smtpout; bh=vbWvuoLw1G7Lynp0lTKmkU iQTXA=; b=cVz7nEySwkPlEYizNbvojomsnVJdJK3mWE2nXfKIFXy7UFGgLxWEAE tQQOpsNfO8KvFVtaYfotZguYBnwqvf2DLXWE5jDJ/Dl/xt4HMvfizlP5ikHm/AbO ubKW4/xVAgBcMUstT4HMxcyhuzPLPBOn2ed6F80zQSalvm7STcjt4= X-Sasl-enc: 7oXcFONHGHto77jPxvrJ6gGlibQ4YovE4zkRlI9eY1Xl 1400674890 Received: from shorty.local (unknown [23.243.199.75]) by mail.messagingengine.com (Postfix) with ESMTPA id BEB5F680118 for ; Wed, 21 May 2014 08:21:30 -0400 (EDT) Received: from dima by shorty.local with local (Exim 4.82) (envelope-from ) id 1Wn5WT-0007KR-ER for bug-gnu-emacs@gnu.org; Wed, 21 May 2014 05:21:29 -0700 From: Dima Kogan To: bug-gnu-emacs@gnu.org Subject: 24.3; [PATCH] Improved diff-mode navigation/manipulation Date: Wed, 21 May 2014 05:21:28 -0700 Message-ID: <87ha4jgw53.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -5.0 (-----) X-Debbugs-Envelope-To: submit X-Mailman-Approved-At: Wed, 21 May 2014 10:59:50 -0400 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -5.0 (-----) --=-=-= Content-Type: text/plain Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From 27d926293402c74eb3d83124e0287dd49cb4543a Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Thu, 15 May 2014 00:24:00 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 77 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 923de9a..d334307 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -593,6 +593,58 @@ next hunk if TRY-HARDER is non-nil; otherwise signal an error." (easy-mmode-define-navigation diff-file diff-file-header-re "file" diff-end-of-file) +(defun diff--wrap-hunk-navigation (isinteractive name orig count) + "I override the default diff-hunk-next/prev implementation by +skipping any lines that are associated with this hunk but precede +the hunk-start marker. For instance, a diff file could contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Here I move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker" + (if (not isinteractive) + (funcall orig count) + + (let ((start (point))) + (let ((hunk-bounds (diff-bounds-of-hunk))) + (goto-char (car hunk-bounds))) + + (funcall orig count) + + (when (not (looking-at diff-hunk-header-re)) + (goto-char start) + (user-error (format "No %s hunk" name)))))) + +(setq diff--hunk-prev-internal (symbol-function 'diff-hunk-prev)) +(defun diff-hunk-prev (&optional count) + "Go to the previous COUNT'th hunk" + (interactive "p") + (diff--wrap-hunk-navigation + (called-interactively-p 'interactive) + "prev" + diff--hunk-prev-internal + count)) + +(setq diff--hunk-next-internal (symbol-function 'diff-hunk-next)) +(defun diff-hunk-next (&optional count) + "Go to the next COUNT'th hunk" + (interactive "p") + (diff--wrap-hunk-navigation + (called-interactively-p 'interactive) + "next" + diff--hunk-next-internal + count)) + + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. The return value is a list (BEG END), which are the hunk's start @@ -602,12 +654,13 @@ point is in a file header, return the bounds of the next hunk." (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1683,8 +1736,9 @@ SRC and DST are the two variants of text as returned by `diff-hunk-text'. SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1693,7 +1747,7 @@ NOPROMPT, if non-nil, means not to prompt the user." ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1802,7 +1856,7 @@ With a prefix argument, REVERSE the hunk." (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) (when diff-advance-after-apply-hunk - (diff-hunk-next)))))) + (call-interactively 'diff-hunk-next)))))) (defun diff-test-hunk (&optional reverse) @@ -1873,14 +1927,15 @@ For use in `add-log-current-defun-function'." (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1967,8 +2022,8 @@ For use in `add-log-current-defun-function'." (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-change))) @@ -1976,7 +2031,7 @@ For use in `add-log-current-defun-function'." (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.0.0.rc0 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Tue Feb 23 21:33:31 2016 Received: (at 17544) by debbugs.gnu.org; 24 Feb 2016 02:33:31 +0000 Received: from localhost ([127.0.0.1]:41898 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84) (envelope-from ) id 1aYPGd-0008FZ-Ap for submit@debbugs.gnu.org; Tue, 23 Feb 2016 21:33:31 -0500 Received: from hermes.netfonds.no ([80.91.224.195]:58853) by debbugs.gnu.org with esmtp (Exim 4.84) (envelope-from ) id 1aYPGc-0008FS-4p for 17544@debbugs.gnu.org; Tue, 23 Feb 2016 21:33:30 -0500 Received: from cpe-60-225-211-161.nsw.bigpond.net.au ([60.225.211.161] helo=mouse) by hermes.netfonds.no with esmtpsa (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1aYPGF-0005eu-EZ; Wed, 24 Feb 2016 03:33:08 +0100 From: Lars Ingebrigtsen To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> Date: Wed, 24 Feb 2016 13:33:03 +1100 In-Reply-To: <87ha4jgw53.fsf@secretsauce.net> (Dima Kogan's message of "Wed, 21 May 2014 05:21:28 -0700") Message-ID: <87si0ion1c.fsf@gnus.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-MailScanner-ID: 1aYPGF-0005eu-EZ X-Netfonds-MailScanner: Found to be clean X-Netfonds-MailScanner-From: larsi@gnus.org MailScanner-NULL-Check: 1456885988.43817@qD5lwyS0Rq69oQwl3jtwUQ X-Spam-Status: No X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 17544 Cc: 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.0 (/) Dima Kogan writes: > Navigation and use of diff buffers had several annoying corner cases that this > patch fixes. These corner cases were largely due to inconsistent treatment of > file headers. Say you have a diff such as this: > > --- aaa > +++ bbb > @@ -52,7 +52,7 @@ > hunk1 > @@ -74,7 +74,7 @@ > hunk2 > --- ccc > +++ ddd > @@ -608,6 +608,6 @@ > hunk3 > @@ -654,7 +654,7 @@ > hunk4 > > The file headers here are the '---' and '+++' lines. With the point on such a > line, hunk operations would sometimes refer to the next hunk and sometimes to > the previous hunk. Most of the time it would be the previous hunk, which is not > what the user would expect. This patch consistently treats such headers as the > next hunk. So with this patch, if the point is on the '--- ccc' line, the point > is seen as referring to hunk3. > > Specific behaviors this fixes are: > > 1. It should be possible to place the point in the middle of a diff buffer, and > press M-k repeatedly to kill hunks in the order they appear in the buffer. With > the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on > hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on > hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. > > 2. Similarly, it should be possible to apply hunks in order. Previously with the > point at the start, C-c C-a would apply the hunk1, then move the point to the > first @@ header, and thus C-c C-a would try to apply the same hunk again. I think this makes sense, and the patch looks good to me. Does it still apply to the Emacs trunk? Anybody got any objections to installing it? It should also have an entry in NEWS, I think. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no From debbugs-submit-bounces@debbugs.gnu.org Sat Sep 03 05:17:31 2016 Received: (at 17544) by debbugs.gnu.org; 3 Sep 2016 09:17:32 +0000 Received: from localhost ([127.0.0.1]:48251 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bg74q-0004FD-31 for submit@debbugs.gnu.org; Sat, 03 Sep 2016 05:17:31 -0400 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:58145) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bg74k-0004F3-Ak for 17544@debbugs.gnu.org; Sat, 03 Sep 2016 05:17:26 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 151B82059A; Sat, 3 Sep 2016 05:17:22 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Sat, 03 Sep 2016 05:17:22 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=ZhEPg +cwqRiBwIxVRcFGQmRKeaA=; b=gaNimn4yJzBAVkN2tkWrmFXaggQEE02Ty7qZK S0Lu5dU3PnyHJrxbf7KYa0raT6sJtsNgwXRZKGLgbSfHCl3jQj7PXAHpc0i0Nrpd P2daKDWVa4NmI17qYPKkXeWrtLD4/paO+9ltQfTtTziTQtQ6a3qxALdnZkoHkgIP 1aBGzc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=ZhEPg+cwqRiBwIxVRcFGQmRKeaA=; b=ZlO2A gycQeATgRyB7cPL/io7gBz/9ppolnCsQ6Y/6hzOZqzvcflV9qzIle1u9M7cOMgGh SbLpqqqdioI9PzTKVWylbmVcSqYSihGb/BIGh9mgpmc3o80+uihSMA0eA8ZFjcHQ QJCfRoeoZsqbKHoNSm1C4QcEXWHiJXElFp/eT8= X-Sasl-enc: 0WE7q4E51WhXt8tVg5W8tfQcZTMUcu2bi4LVEO3ma0Oq 1472894241 Received: from shorty.local (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id B7D84F29D5; Sat, 3 Sep 2016 05:17:21 -0400 (EDT) Received: from dima by shorty.local with local (Exim 4.87) (envelope-from ) id 1bg74h-0000rM-A8; Sat, 03 Sep 2016 02:17:19 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Dima Kogan To: Lars Ingebrigtsen Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87si0ion1c.fsf@gnus.org> Date: Sat, 03 Sep 2016 02:17:19 -0700 Message-ID: <87k2et8hr4.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) Lars Ingebrigtsen writes: > I think this makes sense, and the patch looks good to me. Does it > still apply to the Emacs trunk? Anybody got any objections to > installing it? > > It should also have an entry in NEWS, I think. This is a gentle ping. Can we install this? From debbugs-submit-bounces@debbugs.gnu.org Sat Sep 03 06:14:39 2016 Received: (at 17544) by debbugs.gnu.org; 3 Sep 2016 10:14:39 +0000 Received: from localhost ([127.0.0.1]:48262 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bg7y8-0005a5-DO for submit@debbugs.gnu.org; Sat, 03 Sep 2016 06:14:39 -0400 Received: from eggs.gnu.org ([208.118.235.92]:34308) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bg7y3-0005Zp-Uc for 17544@debbugs.gnu.org; Sat, 03 Sep 2016 06:14:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bg7xt-0007QB-Sy for 17544@debbugs.gnu.org; Sat, 03 Sep 2016 06:14:26 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD autolearn=disabled version=3.3.2 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:38850) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bg7xt-0007Pi-Pa; Sat, 03 Sep 2016 06:14:21 -0400 Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:3393 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bg7xr-00026l-Se; Sat, 03 Sep 2016 06:14:20 -0400 Date: Sat, 03 Sep 2016 13:14:24 +0300 Message-Id: <83a8fpe1dr.fsf@gnu.org> From: Eli Zaretskii To: Dima Kogan In-reply-to: <87k2et8hr4.fsf@secretsauce.net> (message from Dima Kogan on Sat, 03 Sep 2016 02:17:19 -0700) Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-Spam-Score: -6.5 (------) X-Debbugs-Envelope-To: 17544 Cc: larsi@gnus.org, 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Eli Zaretskii Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -6.5 (------) > From: Dima Kogan > Date: Sat, 03 Sep 2016 02:17:19 -0700 > Cc: 17544@debbugs.gnu.org > > Lars Ingebrigtsen writes: > > > I think this makes sense, and the patch looks good to me. Does it > > still apply to the Emacs trunk? Anybody got any objections to > > installing it? > > > > It should also have an entry in NEWS, I think. > > This is a gentle ping. Can we install this? I'd like to hear Stefan's comments. Regardless, the doc string of diff--wrap-hunk-navigation is not according to our coding style, so please fix it. Also, I don't quite understand why you need changes like this one: - (diff-hunk-next)))))) + (call-interactively 'diff-hunk-next)))))) and the whole issue of testing called-interactively-p that goes with it. Can you explain? Thanks. From debbugs-submit-bounces@debbugs.gnu.org Sat Sep 03 07:05:32 2016 Received: (at 17544) by debbugs.gnu.org; 3 Sep 2016 11:05:32 +0000 Received: from localhost ([127.0.0.1]:48269 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bg8lQ-0006pq-AH for submit@debbugs.gnu.org; Sat, 03 Sep 2016 07:05:32 -0400 Received: from mail-out.m-online.net ([212.18.0.10]:33748) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bg8lN-0006ph-QJ for 17544@debbugs.gnu.org; Sat, 03 Sep 2016 07:05:30 -0400 Received: from frontend01.mail.m-online.net (unknown [192.168.8.182]) by mail-out.m-online.net (Postfix) with ESMTP id 3sRCp04S1Lz3hl0x; Sat, 3 Sep 2016 13:05:28 +0200 (CEST) Received: from localhost (dynscan1.mnet-online.de [192.168.6.68]) by mail.m-online.net (Postfix) with ESMTP id 3sRCp03LblzvldX; Sat, 3 Sep 2016 13:05:28 +0200 (CEST) X-Virus-Scanned: amavisd-new at mnet-online.de Received: from mail.mnet-online.de ([192.168.8.182]) by localhost (dynscan1.mail.m-online.net [192.168.6.68]) (amavisd-new, port 10024) with ESMTP id 1YyfKpzuaWOn; Sat, 3 Sep 2016 13:05:27 +0200 (CEST) X-Auth-Info: UMAiUBiXqx40j+YcipEQxFEzr9MffZO1CqzCweNt8CnxUFl81L1b9RJ5ei4oKu06 Received: from igel.home (ppp-88-217-13-82.dynamic.mnet-online.de [88.217.13.82]) by mail.mnet-online.de (Postfix) with ESMTPA; Sat, 3 Sep 2016 13:05:27 +0200 (CEST) Received: by igel.home (Postfix, from userid 1000) id 2B0322C2D31; Sat, 3 Sep 2016 13:05:27 +0200 (CEST) From: Andreas Schwab To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> X-Yow: I'm totally DESPONDENT over the LIBYAN situation and the price of CHICKEN.. Date: Sat, 03 Sep 2016 13:05:27 +0200 In-Reply-To: <87ha4jgw53.fsf@secretsauce.net> (Dima Kogan's message of "Wed, 21 May 2014 05:21:28 -0700") Message-ID: <87eg51xmyw.fsf@linux-m68k.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) On Mai 21 2014, Dima Kogan wrote: > +(setq diff--hunk-prev-internal (symbol-function 'diff-hunk-prev)) > +(defun diff-hunk-prev (&optional count) This will break if diff-mode is reloaded. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." From debbugs-submit-bounces@debbugs.gnu.org Sat Sep 03 17:24:25 2016 Received: (at 17544) by debbugs.gnu.org; 3 Sep 2016 21:24:25 +0000 Received: from localhost ([127.0.0.1]:49002 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bgIQL-0004P1-L0 for submit@debbugs.gnu.org; Sat, 03 Sep 2016 17:24:25 -0400 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:45443) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bgIQJ-0004Os-6x for 17544@debbugs.gnu.org; Sat, 03 Sep 2016 17:24:24 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id B22E3202A9; Sat, 3 Sep 2016 17:24:22 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute5.internal (MEProxy); Sat, 03 Sep 2016 17:24:22 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=DU5xz ZpBL5AxaCA3xXSsQ0AjSZM=; b=p7Ff19RN+DDgeUjWM84mvC98fNRs4VpfQaKx5 ci+dM7U7TlG88sQ/jA5aqHNJWR9JHyBqfIbnKsEN5ttWs3HVDjHF8kRaNENWqvZa Wo8+JOGMCYaPs4v1RAx5o/gcqupW/QGMBX+dKkhWzM2K5wafaWMHND1Joa3N69X0 bYEmzQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=DU5xzZpBL5AxaCA3xXSsQ0AjSZM=; b=WFulU X19HzONrWlt2mMVSzJzNavaifEzbr1BWKpRXHoWw2LO2cnmGJ6l3LJo1ssfZCvxS ClMzUDgaczuO1wUMXlSkyIxyRTbodq/78JJ9j+7wHdTN7KyQdLUr1txP/xQI4APl tNS0RBQa5NpJ9evNZW8bV5GokTlAYiPvOsDoDA= X-Sasl-enc: uVQ1tfjn+4UWryisJycgMC9tkGKBFhlfhFyedEgFouow 1472937862 Received: from shorty.local (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 4E9B7CCE81; Sat, 3 Sep 2016 17:24:22 -0400 (EDT) Received: from dima by shorty.local with local (Exim 4.87) (envelope-from ) id 1bgIQG-00064a-Mu; Sat, 03 Sep 2016 14:24:20 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Dima Kogan To: Eli Zaretskii Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <83a8fpe1dr.fsf@gnu.org> Date: Sat, 03 Sep 2016 14:24:20 -0700 Message-ID: <87h99w8ynv.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) I'd like to get high-level consensus before making any changes. So ... Eli Zaretskii writes: > Regardless, the doc string of diff--wrap-hunk-navigation is not > according to our coding style, so please fix it. OK. > Also, I don't quite understand why you need changes like this one: > > - (diff-hunk-next)))))) > + (call-interactively 'diff-hunk-next)))))) It's been quite a while since I wrote this, and looking at it just now I can't tell why this is necessary. So let's say one can take out this hunk > and the whole issue of testing called-interactively-p that goes with > it. Can you explain? I'm guessing the interactivity checking in diff-hunk-next and diff-hunk-prev was intended to keep scripts working as before. Again, it has been too long to remember specifically. Andreas Schwab writes: > On Mai 21 2014, Dima Kogan wrote: > >> +(setq diff--hunk-prev-internal (symbol-function 'diff-hunk-prev)) >> +(defun diff-hunk-prev (&optional count) > > This will break if diff-mode is reloaded. Will it? diff-hunk-next and -prev are defined just above in the (easy-mmode-define-navigation diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view block. If one reloads diff-mode, wouldn't this easy-mmode-define-navigation block define the old diff-hunk-prev, which then gets redefined by the code in the patch? It would be nicer to define these functions properly the first time instead of (effectively) advising them. But (easy-mmode-define-navigation) allows arbitrary code to be called after the template, but I need new stuff BEFORE it. Better ideas welcome. dima From debbugs-submit-bounces@debbugs.gnu.org Sat Sep 03 23:27:37 2016 Received: (at 17544) by debbugs.gnu.org; 4 Sep 2016 03:27:37 +0000 Received: from localhost ([127.0.0.1]:49052 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bgO5p-0006XN-6q for submit@debbugs.gnu.org; Sat, 03 Sep 2016 23:27:37 -0400 Received: from mail-it0-f49.google.com ([209.85.214.49]:37762) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bgO5o-0006XC-71 for 17544@debbugs.gnu.org; Sat, 03 Sep 2016 23:27:36 -0400 Received: by mail-it0-f49.google.com with SMTP id e124so101033341ith.0 for <17544@debbugs.gnu.org>; Sat, 03 Sep 2016 20:27:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=EIskTG9ZvIOQIpts4+v2EI+nTypRA7bE0QHWvL5dsrM=; b=B+N3qNQQ1j7GeTFnFt/42m2VhgfBlRoNev2IBHKq9wJquctI+xs0OkiTpFmvXeyJV8 X0gYRR2fv138alcmtdehJN0SwIbBWI6EJmcghtybsElh4J80MovvjBBcz3LcgqLCdEg2 sC3mIbDaN0h8O64R0GYY2tcGjSadVN+XW37FQruZeqSNQnIjhW5TGyZcCONZfh/m1j68 Mq4/+wBvlpVbKTgj5COiZme6Wnumx/O+VPh6cuw4CyZHUJSDvJJniP3OCz62/zCrnqhs bcQCakP3kNOYldc80FeCFjOY0mLKnHWF+LVsU3zZGTAtLG9fS7LPeq2NDu6l1Qoz5Gy/ pb7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=EIskTG9ZvIOQIpts4+v2EI+nTypRA7bE0QHWvL5dsrM=; b=X8JneKrLfCLb+mA0qpZzwfyUAKxg4KRlX6k1fPnQFOW2AqkvBBLHJCubj+AL/J9irl dN4o25bZIW2p4sT7tDZmSEm77nHiiXU2qa9u0Ks48bjUOQ2C2vRKkhx+bm/PExpuIWGh UUIMRhiMdAOKC0wMB+fWjftPWMIsXqjH1WySZIuk/qx4jMsDbbPJIxQz8BwVjxInWDA9 Ec83KQMBLlJ5IGITfAPzA7d8/n+pQkR8GKODFD6GRdNwitr+oV4zDYX0evCuTH1duufd iU64x6QjB/5r4k7c7NUHdWvZn+xH4m4A4LNh69llXum6oucoKvGhiRXmXfu6rlFaPM+l hzMQ== X-Gm-Message-State: AE9vXwP450JXPd+w1Vhd6FYgML/C35yVZbSm5sfotO2df1qxZ7zn6gcX4is0fNXvJcVB2w== X-Received: by 10.36.214.193 with SMTP id o184mr14911399itg.5.1472959650666; Sat, 03 Sep 2016 20:27:30 -0700 (PDT) Received: from zony ([45.2.7.130]) by smtp.googlemail.com with ESMTPSA id z125sm3384413itc.0.2016.09.03.20.27.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 03 Sep 2016 20:27:30 -0700 (PDT) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> Date: Sat, 03 Sep 2016 23:27:50 -0400 In-Reply-To: <87h99w8ynv.fsf@secretsauce.net> (Dima Kogan's message of "Sat, 03 Sep 2016 14:24:20 -0700") Message-ID: <87k2es9weh.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) Dima Kogan writes: > >> and the whole issue of testing called-interactively-p that goes with >> it. Can you explain? > > I'm guessing the interactivity checking in diff-hunk-next and > diff-hunk-prev was intended to keep scripts working as before. Again, it > has been too long to remember specifically. IMO, it would make more sense to just define your new commands directly, something like: (defun diff-hunk-next-command (&optional count) "." (interactive "p") (let ((start (point))) (let ((hunk-bounds (diff-bounds-of-hunk))) (goto-char (car hunk-bounds))) (diff-hunk-next count) (when (not (looking-at diff-hunk-header-re)) (goto-char start) (user-error "No next hunk")))) And then just give the *binding* of `diff-hunk-next' to `diff-hunk-next-command'. No need to make a higher order wrapper function just for defining 2 functions. From debbugs-submit-bounces@debbugs.gnu.org Wed Sep 07 03:15:00 2016 Received: (at 17544) by debbugs.gnu.org; 7 Sep 2016 07:15:00 +0000 Received: from localhost ([127.0.0.1]:51872 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bhX4W-000311-6H for submit@debbugs.gnu.org; Wed, 07 Sep 2016 03:15:00 -0400 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:54374) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bhX4V-00030u-Bh for 17544@debbugs.gnu.org; Wed, 07 Sep 2016 03:14:59 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id ED25D2037A; Wed, 7 Sep 2016 03:14:58 -0400 (EDT) Received: from frontend2 ([10.202.2.161]) by compute5.internal (MEProxy); Wed, 07 Sep 2016 03:14:59 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=t2DoF AeI09uQvpN/Z4A7hiiDAks=; b=XK2kuFUizCg461FX3Ev7HjCtL9+UWtfZ/7ny5 wcs0L3dK150J/neP1H9TRTYWeSq/s6MbSSDtfF2zSsxaWoZwVnFbTXQEy/bslvO4 UPm+8ImxVJUgOFrtEJoz5thliP0pMgI3E/Rs1GEI1Or8zSoqXg2EIAyl1PToa0v2 9AMnaE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=t2DoFAeI09uQvpN/Z4A7hiiDAks=; b=taEr3 TWc6g4FqPgmeD7StyG6Ltlq/zEQcs58oikAlNLt38Ltwwsm9oazsKPNP5Db385jb cuhYznzBBeG1NYqEd45ODzNxalKoGmXdiJGf9y15mZCXk2aTN8XuHFDJsU5TKLpX y3mUuexD9zQxLdaJDjdNgApw5DAp7Ow6W/TuaE= X-Sasl-enc: qYJmKAXZUOoC5iO03sUp/f2phqocuEBx6t5onGDujhQp 1473232498 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 95CD8CCE75; Wed, 7 Sep 2016 03:14:58 -0400 (EDT) Received: from [::1] (helo=scrawny) by scrawny with esmtp (Exim 4.87) (envelope-from ) id 1bhX4T-0002Xp-8S; Wed, 07 Sep 2016 00:14:57 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87k2es9weh.fsf@users.sourceforge.net> Date: Wed, 07 Sep 2016 00:14:56 -0700 Message-ID: <87oa4018r3.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) npostavs@users.sourceforge.net writes: > Dima Kogan writes: > >> >>> and the whole issue of testing called-interactively-p that goes with >>> it. Can you explain? >> >> I'm guessing the interactivity checking in diff-hunk-next and >> diff-hunk-prev was intended to keep scripts working as before. Again, it >> has been too long to remember specifically. > > IMO, it would make more sense to just define your new commands directly, > something like: > > (defun diff-hunk-next-command (&optional count) > "." > (interactive "p") > (let ((start (point))) > (let ((hunk-bounds (diff-bounds-of-hunk))) > (goto-char (car hunk-bounds))) > (diff-hunk-next count) > (when (not (looking-at diff-hunk-header-re)) > (goto-char start) > (user-error "No next hunk")))) > > And then just give the *binding* of `diff-hunk-next' to > `diff-hunk-next-command'. No need to make a higher order wrapper > function just for defining 2 functions. I've no problems with this. If I address this and the other comments, we can talk about merging this? Thanks From debbugs-submit-bounces@debbugs.gnu.org Mon Sep 12 23:56:26 2016 Received: (at 17544) by debbugs.gnu.org; 13 Sep 2016 03:56:26 +0000 Received: from localhost ([127.0.0.1]:58102 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bjepe-0000iq-0v for submit@debbugs.gnu.org; Mon, 12 Sep 2016 23:56:26 -0400 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:49297) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bjepc-0000ii-OZ for 17544@debbugs.gnu.org; Mon, 12 Sep 2016 23:56:25 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 462B6206F2; Mon, 12 Sep 2016 23:56:24 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Mon, 12 Sep 2016 23:56:24 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=/K5tm EjWqzKMMtz+vYQo6b6bgks=; b=Q+4NYb0SzoQG9liSprFwP8eEdVabnHLRB7nXm 6LXYa+jRncL0PNrNeK92sstfzFgPPYXyagi7tv4fBN/I3gE6cO6sD0Rf00U9b2sF jmin8qCzbxmePrHZjIo9bhavbJiFQc+eEk7mGi6xO0SQr0tqdhVuKyz4jzy8ftj4 hsdbSc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=/K5tmEjWqzKMMtz+vYQo6b6bgks=; b=e9bEG GjFe5wewnUSQuD/vjox9qzkqLW1I6A9wNRbtajoktkBcr8bU7VreVnu6UL9Vz33b JhOF/wYX5+nxd9WJi1iPE6Iev6iUwY9AOOCeZdtP9plxsimKKCSWQpL9BPdq7GwE SRbBnZhBl4skpQM7zbDwz3exkBLCypDaNMEU/g= X-Sasl-enc: MKXtA+p+DOVDCJ7FTed4+lYbecOMB3oQWPlVsb5KEi0i 1473738983 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id E5FCCF29CF; Mon, 12 Sep 2016 23:56:23 -0400 (EDT) Received: from [::1] (helo=scrawny) by scrawny with esmtp (Exim 4.87) (envelope-from ) id 1bjepa-00041j-FV; Mon, 12 Sep 2016 20:56:22 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87k2es9weh.fsf@users.sourceforge.net> Date: Mon, 12 Sep 2016 20:56:22 -0700 Message-ID: <87fup41mhl.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) npostavs@users.sourceforge.net writes: > Dima Kogan writes: c> >> >>> and the whole issue of testing called-interactively-p that goes with >>> it. Can you explain? >> >> I'm guessing the interactivity checking in diff-hunk-next and >> diff-hunk-prev was intended to keep scripts working as before. Again, it >> has been too long to remember specifically. > > IMO, it would make more sense to just define your new commands directly, > something like: > > (defun diff-hunk-next-command (&optional count) > "." > (interactive "p") > (let ((start (point))) > (let ((hunk-bounds (diff-bounds-of-hunk))) > (goto-char (car hunk-bounds))) > (diff-hunk-next count) > (when (not (looking-at diff-hunk-header-re)) > (goto-char start) > (user-error "No next hunk")))) > > And then just give the *binding* of `diff-hunk-next' to > `diff-hunk-next-command'. No need to make a higher order wrapper > function just for defining 2 functions. From debbugs-submit-bounces@debbugs.gnu.org Wed Sep 14 18:31:16 2016 Received: (at 17544) by debbugs.gnu.org; 14 Sep 2016 22:31:16 +0000 Received: from localhost ([127.0.0.1]:59893 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bkIi3-0002mR-Kg for submit@debbugs.gnu.org; Wed, 14 Sep 2016 18:31:16 -0400 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:55579) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bkIi0-0002mI-Ue for 17544@debbugs.gnu.org; Wed, 14 Sep 2016 18:31:14 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 5157B21A2C; Wed, 14 Sep 2016 18:31:12 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Wed, 14 Sep 2016 18:31:12 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=15j/B MBNSSXOGr+71/iaje/Xug8=; b=brlFyA4c9d5rtOh2bhIxi/RmPcWdwbUryp6e5 7Ulrd8AwhsH4qKBrR+egFaQE4TcZ/B+Ek242IB8Wv0MnR+Mz3p9kEgz0WTlSZyRa LUuPplthKL0DWATYCiHd2jeaDXWaylbCtxBomqDmZYYYpYEMWKq9qepy6BrgBb9T ccWwKQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=15j/BMBNSSXOGr+71/iaje/Xug8=; b=OT1fh zWHNUplhEtyEIROO1dgkjs2WDXroR0I9NtLe7j6rcJK2ZwwxsmdNtNbBUam14dCx iGdPOUd1Lj2kuX00DBjPBaTyFcBFBsOynN4dNA9wCH2lB62e6ZxAcAissbZIfAA4 yuElHYKpEKfCeBghgZ6hHKkm7e/IBK4BLqJOms= X-Sasl-enc: SHWq1loZmgx+icltWJ3x8WHvWL1yHuGd90RtdA/3wmpL 1473892271 Received: from shorty.local (unknown [128.149.110.210]) by mail.messagingengine.com (Postfix) with ESMTPA id E68E4F2C78; Wed, 14 Sep 2016 18:31:11 -0400 (EDT) Received: from dima by shorty.local with local (Exim 4.87) (envelope-from ) id 1bkIhy-0003BL-Ru; Wed, 14 Sep 2016 15:31:10 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87oa4018r3.fsf@secretsauce.net> Date: Wed, 14 Sep 2016 15:31:10 -0700 Message-ID: <87twdi6rm9.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain Here's a revised patch. I've been running with this for a few days, and it feels delightful. Changes from the last time: - Removed the unnecessary (call-interactively ...) pointed out by Eli - Instead of using (easy-mmode-define-navigation) to create diff-{hunk,file}-{next,prev} and then overriding that definition, I now have (easy-mmode-define-navigation) create diff--internal-{hunk,file}-{next,prev}, and then my own (diff-{hunk,file}-{next,prev}) call those functions - I now handle file navigation in addition to hunk navigation --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From 8070ca643f5467c3dcfeb8d45b76a897b347d518 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 110 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 98 insertions(+), 12 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..6716f39 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ next hunk if TRY-HARDER is non-nil; otherwise signal an error." ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,88 @@ next hunk if TRY-HARDER is non-nil; otherwise signal an error." (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (isinteractive + what orig + header-re goto-start-func count) + "I override the default diff-{hunk,file}-{next,prev} +implementation by skipping any lines that are associated with +this hunk/file but precede the hunk-start marker. For instance, a +diff file could contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Here I move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker" + (if (not isinteractive) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; I trap the error + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +(defun diff-hunk-prev (&optional count) + "Go to the previous COUNT'th hunk" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(defun diff-hunk-next (&optional count) + "Go to the next COUNT'th hunk" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "next hunk" + 'diff--internal-hunk-next + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(defun diff-file-prev (&optional count) + "Go to the previous COUNT'th file" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count) + "Go to the next COUNT'th file" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +662,13 @@ point is in a file header, return the bounds of the next hunk." (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1747,9 @@ SRC and DST are the two variants of text as returned by `diff-hunk-text'. SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1758,7 @@ NOPROMPT, if non-nil, means not to prompt the user." ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1784,6 +1867,8 @@ With a prefix argument, REVERSE the hunk." (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) (when diff-advance-after-apply-hunk + ;; old option: + ;; (call-interactively 'diff-hunk-next)))))) (diff-hunk-next)))))) @@ -1865,14 +1950,15 @@ For use in `add-log-current-defun-function'." (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2045,8 @@ For use in `add-log-current-defun-function'." (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2054,7 @@ For use in `add-log-current-defun-function'." (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.8.1 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Fri Sep 23 03:22:42 2016 Received: (at 17544) by debbugs.gnu.org; 23 Sep 2016 07:22:42 +0000 Received: from localhost ([127.0.0.1]:32999 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bnKok-000544-DR for submit@debbugs.gnu.org; Fri, 23 Sep 2016 03:22:42 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:48419) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bnKoi-00053w-Lp for 17544@debbugs.gnu.org; Fri, 23 Sep 2016 03:22:41 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 39BE12040C; Fri, 23 Sep 2016 03:22:40 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Fri, 23 Sep 2016 03:22:40 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=F10kJ xIg4ySGqjX2i1g+TRXGZqM=; b=ljT0qRjAbEXACtCinhqKq96FsgE2b5MPlQw/X 11u64LvnO83xzjBT+RDSp8qazIsSge7x1lP65eyVbcnlyl6NgSB/rEpRlPa7qFdy Edj3IFxa8c4BtXE0nyBUUl4vJkNwo4uC9CsxcxNCkgsm3N7yy53uTwyntJt+haJI x6R8TE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=F10kJxIg4ySGqjX2i1g+TRXGZqM=; b=BErt/ dzdCiKfqhOznNfzfFguW79bo/C2F3hxz2WFTyHYteS6RNq1lIlUAmu2TLxFITuiC WPv4GthTB54WDOBA8XxUZ+YHvbSdY+YW5HcTfSDQTwQdjBKohpVwXhiBFBAceLpz kxmUlza1T0sVv7D+1BgkReRzcKbPWd0fPR/y9c= X-Sasl-enc: 7qgMcXBXYZWCGnthfn67lpqj135zGijwD2hNU7jDTnAr 1474615359 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id AB2EEF2985; Fri, 23 Sep 2016 03:22:39 -0400 (EDT) Received: from dima by scrawny with local (Exim 4.87) (envelope-from ) id 1bnKog-0001NA-1w; Fri, 23 Sep 2016 00:22:38 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> User-agent: mu4e 0.9.17; emacs 25.1.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87twdi6rm9.fsf@secretsauce.net> Date: Fri, 23 Sep 2016 00:22:38 -0700 Message-ID: <87h997nkqp.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain 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. This patch puts the (call-interactively) back in, and explains the rationale. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From 21964781a87b615cdab65ab2643deaac8dbaa406 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 115 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 102 insertions(+), 13 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..b37ceec 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ diff--auto-refine-data ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,88 @@ diff--auto-refine-data (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (isinteractive + what orig + header-re goto-start-func count) + "I override the default diff-{hunk,file}-{next,prev} +implementation by skipping any lines that are associated with +this hunk/file but precede the hunk-start marker. For instance, a +diff file could contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Here I move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker" + (if (not isinteractive) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; I trap the error + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +(defun diff-hunk-prev (&optional count) + "Go to the previous COUNT'th hunk" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(defun diff-hunk-next (&optional count) + "Go to the next COUNT'th hunk" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "next hunk" + 'diff--internal-hunk-next + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(defun diff-file-prev (&optional count) + "Go to the previous COUNT'th file" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count) + "Go to the next COUNT'th file" + (interactive "p") + (diff--wrap-navigation + (called-interactively-p 'interactive) + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +662,13 @@ diff-bounds-of-hunk (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1747,9 @@ diff-find-source-location SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1758,7 @@ diff-find-source-location ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1783,8 +1866,13 @@ diff-apply-hunk ;; Display BUF in a window (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) + + ;; 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)))))) + (call-interactively 'diff-hunk-next)))))) (defun diff-test-hunk (&optional reverse) @@ -1865,14 +1953,15 @@ diff-current-defun (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2048,8 @@ diff-refine-hunk (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2057,7 @@ diff-refine-hunk (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.8.0.rc3 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sat Oct 22 11:46:59 2016 Received: (at 17544) by debbugs.gnu.org; 22 Oct 2016 15:46:59 +0000 Received: from localhost ([127.0.0.1]:46501 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bxyVf-0000wS-BH for submit@debbugs.gnu.org; Sat, 22 Oct 2016 11:46:59 -0400 Received: from mail-it0-f68.google.com ([209.85.214.68]:34214) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bxyVd-0000wF-NN for 17544@debbugs.gnu.org; Sat, 22 Oct 2016 11:46:58 -0400 Received: by mail-it0-f68.google.com with SMTP id e203so3622873itc.1 for <17544@debbugs.gnu.org>; Sat, 22 Oct 2016 08:46:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=WK2esMgWh56KVD7Sg+8m8IlI8f0TyDEMZ5QLPgOpx/g=; b=oIVRLMU2BMmhX9v4emmfHT0mjVNNxvFyCwayFYkv7bHVPmmBgy15GHWMhSCAnmIRo2 480pnBXmsUiNWp34avhI7FWS/VZjzin1Era5Cv+SrdmmidXBsYpH0urGkT+9TPWvOVXU ZvzyABzzmkW3uBDqhFlanOmCWlwBsx8HLu4olNU7i5ymQL4V5d+YJ4j7q9d/bLWkT5pd /vEdz1To9K/GAa8JXdV2IZfbT8WcRQ7Rzd5UpXoMpILRZe9Uomf0ub9d5pSKzULK3L4E 1dy61rRioZ+NgRx2PbeYOHGsCl1jz1Efr16ePOXGcNbWRNapwkpUHfC44Y3uDTq3/ndR MzdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=WK2esMgWh56KVD7Sg+8m8IlI8f0TyDEMZ5QLPgOpx/g=; b=SjXDREY5ViX9ZpnKOB4IDQV9MSfc9EgeoNjQKl4GWVx5Qupdf4HKW1HO69o55ByEgb 8kocKjt0R774Emvpy2noOj2Mrc2eGv79yz5EE4LDL4lIrCKaoPUMT054ELSOiDSMtHZw RhfSUrjedDYaEau/UMvWlp9B26uZT8c60p3yiab5KzHBNxpocDK/NUVeJCRLhEpxYjl5 0guGcQTHXwWpyJJeUfrUmbXa35d4wQ7WKyMORRtwIK0Em4re8jbMEuVr0O+wY1XCDbX7 yJDz0eqlB17kBqMLkOaiUH/rqje4hRfDCi7wtrtQKEe0FjEmWyU8IRdiyShp/kq04nue ZkGw== X-Gm-Message-State: ABUngveFu9iTGdi590DLBKLmsgQyqMcxEZX8NS46Afl55bgZfZUMNHW2aJqLZitKH3qaAQ== X-Received: by 10.36.68.65 with SMTP id o62mr6036285ita.82.1477151212082; Sat, 22 Oct 2016 08:46:52 -0700 (PDT) Received: from zony ([45.2.7.130]) by smtp.googlemail.com with ESMTPSA id 80sm3970950iot.36.2016.10.22.08.46.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 22 Oct 2016 08:46:51 -0700 (PDT) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> Date: Sat, 22 Oct 2016 11:47:29 -0400 In-Reply-To: <87h997nkqp.fsf@secretsauce.net> (Dima Kogan's message of "Fri, 23 Sep 2016 00:22:38 -0700") Message-ID: <874m44tmge.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.0 (/) Dima Kogan 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. From debbugs-submit-bounces@debbugs.gnu.org Sat Oct 22 21:44:20 2016 Received: (at 17544) by debbugs.gnu.org; 23 Oct 2016 01:44:20 +0000 Received: from localhost ([127.0.0.1]:46697 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1by7pk-00009E-9i for submit@debbugs.gnu.org; Sat, 22 Oct 2016 21:44:20 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:55488) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1by7pi-000096-Jp for 17544@debbugs.gnu.org; Sat, 22 Oct 2016 21:44:18 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id DBF1F2079C; Sat, 22 Oct 2016 21:44:17 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute5.internal (MEProxy); Sat, 22 Oct 2016 21:44:17 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=DfLpf F6h5lzlHXcsnp62aiOhcUk=; b=Ad+mLiFd+RmlV6acUjU/avBrwCEtVZXAsqSN5 n1zm0mLhb3RLAjnk8QO/t2v74c2+92EgiSawKMrqBikdmx/CMeflSZVo2XBkAxjR M+nfIfX6a2xPft2nQhXpRe9lPX4xHQWXcBB7AnyVGNjg33CW/yWMVstelVrBHpND 6mBKTw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=DfLpfF6h5lzlHXcsnp62aiOhcUk=; b=TCM31 IYKVzewAEfCpXsdkjwWwO2DNm83CDrRGlYy5k8TLUA1+MhIMZJbxLe6riNsGADdC dPW2f1aSHgok52cun/ZSgprnJl3+Km1yJqECfw0Rm1fDeAg9BlPKrUkyXnmYL0ox FuqdtNeSxraYGjO3iuz6XnqeL8i+cfaGQSOrek= X-Sasl-enc: HsERxI2BH8AP09TBklnmgyaU4WUe7CoQ9iaekYOl+p1T 1477187057 Received: from shorty.local (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 93782F29C9; Sat, 22 Oct 2016 21:44:17 -0400 (EDT) Received: from dima by shorty.local with local (Exim 4.87) (envelope-from ) id 1by7pf-0003jC-TV; Sat, 22 Oct 2016 18:44:15 -0700 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 26.0.50.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <874m44tmge.fsf@users.sourceforge.net> Date: Sat, 22 Oct 2016 18:44:15 -0700 Message-ID: <87h983rg9c.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) npostavs@users.sourceforge.net writes: > Dima Kogan 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. Am I wrong to want to preserve existing behavior for elisp code? If so, then the entire old path can simply go away unconditionally. From debbugs-submit-bounces@debbugs.gnu.org Sat Oct 22 22:48:50 2016 Received: (at 17544) by debbugs.gnu.org; 23 Oct 2016 02:48:51 +0000 Received: from localhost ([127.0.0.1]:46711 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1by8qA-0003Mn-O3 for submit@debbugs.gnu.org; Sat, 22 Oct 2016 22:48:50 -0400 Received: from mail-it0-f47.google.com ([209.85.214.47]:37383) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1by8q9-0003Mb-5c for 17544@debbugs.gnu.org; Sat, 22 Oct 2016 22:48:49 -0400 Received: by mail-it0-f47.google.com with SMTP id m138so80401750itm.0 for <17544@debbugs.gnu.org>; Sat, 22 Oct 2016 19:48:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=1cvKZZULAVlA8eb/+EieI9sje1Yi1eBygKDg2Kcri5k=; b=L54h/R9MfXE8EBq0DKnKTpemaHI1hA187v12AKnZbUdqv0cq1suzhCR518xfxZ7UBO RjL3xBd+6VdRRo9ag2uB/dh581qIoul0cZthDEKYzJqf7yQKQzOlSsL3otW7s3IN8K8w okJYjcOXNVKH+Dhe606oYPac83giRBPtt7YpW8eNKV+rZ7T2F6+yTcKMJWYbnoDvf75U 6nmiXph7sqZy25/zm5SeKaVnZ/cSySWhs9QDNq9z2kul9eZv54qMYMfkQXG/OQzsPOim +g0TfBNwKn4CTZviwPgYmsEmSV1QaGpMj90njUev0+f1hirKO+7fvIM8LAOp7OaNp0Jt xlcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=1cvKZZULAVlA8eb/+EieI9sje1Yi1eBygKDg2Kcri5k=; b=CxtZSUf6mH0b37ceZY1TwgkJPe7EzxIYd8b2wiA6aMs8C4kUfNj5+oK/BTPkamjN5z ORIiUr67siQ1OBWaG7Znb49hU/VlHTYWdhJbu/DDizyTEIHoU9trB0camRt9pJR5qVv6 2o2PhRhPEqF5CJX6kGIEMsdO0rnpJNWWCwnMQM8H2vgF2Sb5ers5ILQk8iJU6HFFbywt TWJfOAZhPxGap+pn9xX5kOgi6JCWVn6rjZBLOd7GJSafTtxTFDVc2nReI4B4FCr4zpnd 91fyF9bY4kWmEzYCgyEBXKrN8JGxOiF/l7dcsQSS4ab26J/bn7RKfzRnOuYAHOYdvro6 LMrg== X-Gm-Message-State: ABUngvdlTXI3NvgzXPHzNrsxME5tEeMEdEZBBiG1w09HYZPFWjvf2sujMoZLskue3XRkog== X-Received: by 10.107.138.65 with SMTP id m62mr9372340iod.213.1477190923523; Sat, 22 Oct 2016 19:48:43 -0700 (PDT) Received: from zony ([45.2.7.130]) by smtp.googlemail.com with ESMTPSA id i102sm4814540iod.10.2016.10.22.19.48.42 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 22 Oct 2016 19:48:43 -0700 (PDT) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> Date: Sat, 22 Oct 2016 22:49:21 -0400 In-Reply-To: <87h983rg9c.fsf@secretsauce.net> (Dima Kogan's message of "Sat, 22 Oct 2016 18:44:15 -0700") Message-ID: <87mvhvsrta.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) Dima Kogan writes: > npostavs@users.sourceforge.net writes: > >> Dima Kogan 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? From debbugs-submit-bounces@debbugs.gnu.org Sun Nov 06 21:27:03 2016 Received: (at 17544) by debbugs.gnu.org; 7 Nov 2016 02:27:03 +0000 Received: from localhost ([127.0.0.1]:47341 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c3ZeI-0005S3-VE for submit@debbugs.gnu.org; Sun, 06 Nov 2016 21:27:03 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:54114) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c3ZeG-0005Rd-Im for 17544@debbugs.gnu.org; Sun, 06 Nov 2016 21:27:01 -0500 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 596232068C; Sun, 6 Nov 2016 21:27:00 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute5.internal (MEProxy); Sun, 06 Nov 2016 21:27:00 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=V5cVhrxWeroSReZjnHyd7ZzHQFY=; b=vbR8EF ENAofu+WSu/1k13TkdNl0xie+rUyxtqCu4B3o0s9I+L7GHF4wCxL67lrqZjFrbd6 X4GoBKonckCA9Jw508UMomKguFWkrlfLNQ4d41KJ0H1Cf826+xO7hS2+tp9SRUju iKTkclbcNcxOhnTydYkPlipRddXx5XlZ7qD+k= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=smtpout; bh=V5cVhrxWeroSRe ZjnHyd7ZzHQFY=; b=TliO57kwfDqPSjFMNblTpLpTkCAqUQkFokSHkQbmG0N8Yz F4dHKENzL+ZgbNGHfwrHfW1nmVwgnrwjZ5y1MPUAUFXnp6fen1bEfGMamcUyoMdK pgz7y1y++KLdo/tiSj1zM1GCoAxeqDA1RjcmN8ny/G5Q+U556iY+YR4h37eHw= X-ME-Sender: X-Sasl-enc: zmcv24EmnnTbqzHWLTgR8gKbD7dWTOAepl5z7IW5HL8t 1478485618 Received: from shorty.local (c-73-140-59-251.hsd1.wa.comcast.net [73.140.59.251]) by mail.messagingengine.com (Postfix) with ESMTPA id 407D1CCF2D; Sun, 6 Nov 2016 21:26:58 -0500 (EST) Received: from dima by shorty.local with local (Exim 4.87) (envelope-from ) id 1c3ZeA-0007lh-RM; Sun, 06 Nov 2016 18:26:54 -0800 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 26.0.50.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation Message-ID: <87wpgge0kl.fsf@secretsauce.net> In-reply-to: <87mvhvsrta.fsf@users.sourceforge.net> Date: Sun, 06 Nov 2016 18:26:54 -0800 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain npostavs@users.sourceforge.net writes: > Dima Kogan 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 > 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) Yes. Agreed. Revised patch attached. I'll try to respond to any comments quickly so that we all can remember what this is about. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From 4cd4f839dc840493ca14ff13a4f27cedca12a562 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 115 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 102 insertions(+), 13 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..16d3f46 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ diff--auto-refine-data ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,88 @@ diff--auto-refine-data (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (skip-hunk-start + what orig + header-re goto-start-func count) + "I override the default diff-{hunk,file}-{next,prev} +implementation by skipping any lines that are associated with +this hunk/file but precede the hunk-start marker. For instance, a +diff file could contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Here I move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker" + (if (not skip-hunk-start) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; I trap the error + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +(defun diff-hunk-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th hunk" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(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)) + +(defun diff-file-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th file" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count skip-hunk-start) + "Go to the next COUNT'th file" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +662,13 @@ diff-bounds-of-hunk (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1747,9 @@ diff-find-source-location SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1758,7 @@ diff-find-source-location ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1783,8 +1866,13 @@ diff-apply-hunk ;; Display BUF in a window (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) + + ;; 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)))))) (defun diff-test-hunk (&optional reverse) @@ -1865,14 +1953,15 @@ diff-current-defun (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2048,8 @@ diff-refine-hunk (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2057,7 @@ diff-refine-hunk (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.10.1 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Mon Nov 14 22:30:36 2016 Received: (at 17544) by debbugs.gnu.org; 15 Nov 2016 03:30:36 +0000 Received: from localhost ([127.0.0.1]:57564 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c6USC-0005cr-JA for submit@debbugs.gnu.org; Mon, 14 Nov 2016 22:30:36 -0500 Received: from mail-it0-f68.google.com ([209.85.214.68]:33877) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c6USA-0005ce-Oa for 17544@debbugs.gnu.org; Mon, 14 Nov 2016 22:30:35 -0500 Received: by mail-it0-f68.google.com with SMTP id q124so19185497itd.1 for <17544@debbugs.gnu.org>; Mon, 14 Nov 2016 19:30:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=LqHHxQg1l/iBxYwZ9LKEDFnIOjY5vQ59KBKTHUxX8rs=; b=M0zfpqJo68xpE/gf2t/d5wMg0G0P+FsVlPUMWlJ/qoWKyaneREMh3kVoSi0u0D8JaT uTldthcM7cJnmjdv9ltU9ap6vW+OJ0uTsca8YYfltUJCrKlUBtZ2Wf+WJAt6MzwoRsYw jdwUmgOV+SrQuPDSnaEcUSlur0cSoHruLgSEk/AyBeTOZ3qZc3xyge6bEOVssAkCnbZq fFbSGKaUds5LMmIomtr87nXrBf2TrleH0UtjFdEac4g9KM2uFQ8I7apPO7q4i9/hG7Kq dolcHxb3/91DnHHkXlxL58ZRERQBHlb5SWUqcsiuaRv1aiE1CqccLIDlVA61SO2e1x5d iEIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=LqHHxQg1l/iBxYwZ9LKEDFnIOjY5vQ59KBKTHUxX8rs=; b=FcSRaVH1BbbghrPH/9wb+jQmUQUoDX6f+t7csTkNcjE6RtOkeFpwnALbO5VgthhW3M 5Rab4ZTb319g3jmaTIeSqAdDjjXm94eRJGo9ZHjoX9ftJUGR+J1cLB+0ruNX416snTML CQY4dikoSgeSxMI1Yk01hR4VLyMYq9y/I7QXvNErOa5YDitR1BJ/Az2Q4+KKIkhvf6Ia Yy3x25DvUkn6VsOWhbbdRvOH/R/Znd1dCBD+eo6yUDkTgCQnmKeuqdu5tavJWd/jkOum HXYwJmQd1DEJt+B6T7D+cjM4rEFR58YkPD/YGbOG7TVap/nl+pIRsOV6YTp6tdr5122u JvpA== X-Gm-Message-State: ABUngvc6Y01mxqULhpJL/pYMwJLlXxqpTJlXFcHUIC603jKtp2G0X99Xz+dwBPkzM+W2hw== X-Received: by 10.107.199.66 with SMTP id x63mr28710260iof.97.1479180629130; Mon, 14 Nov 2016 19:30:29 -0800 (PST) Received: from zony ([45.2.7.65]) by smtp.googlemail.com with ESMTPSA id t129sm10670290iod.11.2016.11.14.19.30.28 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 14 Nov 2016 19:30:28 -0800 (PST) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> Date: Mon, 14 Nov 2016 22:31:17 -0500 In-Reply-To: <87wpgge0kl.fsf@secretsauce.net> (Dima Kogan's message of "Sun, 06 Nov 2016 18:26:54 -0800") Message-ID: <87twb9l8qi.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.5 (/) Dima Kogan writes: > npostavs@users.sourceforge.net writes: > >> Dima Kogan 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. From debbugs-submit-bounces@debbugs.gnu.org Wed Nov 16 23:15:54 2016 Received: (at 17544) by debbugs.gnu.org; 17 Nov 2016 04:15:54 +0000 Received: from localhost ([127.0.0.1]:60049 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c7E77-0002uR-RT for submit@debbugs.gnu.org; Wed, 16 Nov 2016 23:15:54 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:49055) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c7E75-0002uI-MM for 17544@debbugs.gnu.org; Wed, 16 Nov 2016 23:15:52 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 60442207EA; Wed, 16 Nov 2016 23:15:51 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Wed, 16 Nov 2016 23:15:51 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=sz8l4QQLgx92gAyjEretTWbFzWM=; b=Hg9Vsg xwHQA8cRib7ct3wAAI9uSY9k6y/AuCMZpwY/W3u27qTM9QeskkbkJydnIdU+oV4i Xfyg9mA1MMdtfHyWP8cQtGXNhe0qDwuJjAN9cAwuDFgNfJpoACGU8uIlx7ojs5A5 aIZwDzucwkEnQIRLGvrACmU5X/JWw7fz69ZdE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=smtpout; bh=sz8l4QQLgx92gA yjEretTWbFzWM=; b=BxiRCLNibQv0um4GK0pLxCmGFhkSgXMZStXfXEfKlhU4a0 yqqDz3m/7wsfSgJprNSNdVfXAVdLQ9FPITaMgyvQbONmyZ6AiYXTN7818ObtonIT ERrqpyV/W5ZoPocBHuDBWsI2hN2dUwe47mQ7APsDGqpNIvbCfwu/XHCsYiIaM= X-ME-Sender: X-Sasl-enc: ZObWn7FO28CJzQhW0bX8kbd6YPk/nKoFzMNTSV0BEsKP 1479356150 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id D7A3124454; Wed, 16 Nov 2016 23:15:50 -0500 (EST) Received: from dima by scrawny with local (Exim 4.87) (envelope-from ) id 1c7E73-0004jk-C5; Wed, 16 Nov 2016 20:15:49 -0800 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 26.0.50.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation Message-ID: <878tsihhhl.fsf@secretsauce.net> In-reply-to: <87twb9l8qi.fsf@users.sourceforge.net> Date: Wed, 16 Nov 2016 20:15:34 -0800 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain New patch attached. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From b02085abd998d2c1e1519973964295fb5b798e0c Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 117 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 104 insertions(+), 13 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..4f05b0c 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ diff--auto-refine-data ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,88 @@ diff--auto-refine-data (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (skip-hunk-start + what orig + header-re goto-start-func count) + "I override the default diff-{hunk,file}-{next,prev} +implementation by skipping any lines that are associated with +this hunk/file but precede the hunk-start marker. For instance, a +diff file could contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Here I move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker" + (if (not skip-hunk-start) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; I trap the error + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +(defun diff-hunk-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th hunk" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(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)) + +(defun diff-file-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th file" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count skip-hunk-start) + "Go to the next COUNT'th file" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +662,13 @@ diff-bounds-of-hunk (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1747,9 @@ diff-find-source-location SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1758,7 @@ diff-find-source-location ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1783,8 +1866,15 @@ diff-apply-hunk ;; Display BUF in a window (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) + + ;; I advance to the next hunk with skip-hunk-start set to t + ;; because I want the behavior of moving to the next logical + ;; hunk, not the legacy behavior where were would sometimes stay + ;; on the curent hunk. This is the behavior we get when + ;; navigating through hunks interactively, and we want it when + ;; applying hunks too. See http://debbugs.gnu.org/17544 (when diff-advance-after-apply-hunk - (diff-hunk-next)))))) + (diff-hunk-next nil t)))))) (defun diff-test-hunk (&optional reverse) @@ -1865,14 +1955,15 @@ diff-current-defun (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2050,8 @@ diff-refine-hunk (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2059,7 @@ diff-refine-hunk (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.8.0.rc3 --=-=-= Content-Type: text/plain npostavs@users.sourceforge.net writes: > Dima Kogan writes: > >> npostavs@users.sourceforge.net writes: >> >> 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. This file used tabs in some places and spaces in others. Indentation updated to match. >> + >> + ;; 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. done >> (hunk (delete-and-extract-region >> - (point) (save-excursion (diff-end-of-hunk) (point)))) >> + (point) (cadr hunk-bounds))) > > Indentation looks off here. Same as above. --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Wed Nov 16 23:32:41 2016 Received: (at 17544) by debbugs.gnu.org; 17 Nov 2016 04:32:41 +0000 Received: from localhost ([127.0.0.1]:60053 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c7ENN-0003Ld-Ir for submit@debbugs.gnu.org; Wed, 16 Nov 2016 23:32:41 -0500 Received: from mail-it0-f66.google.com ([209.85.214.66]:34760) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c7ENL-0003LO-MY for 17544@debbugs.gnu.org; Wed, 16 Nov 2016 23:32:40 -0500 Received: by mail-it0-f66.google.com with SMTP id o1so12380544ito.1 for <17544@debbugs.gnu.org>; Wed, 16 Nov 2016 20:32:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=xsuXY8vhSMEbWJGhREr+fl4UvWb8qIXSRM77CwkJJsQ=; b=tYPdEkHWihyKZV1MM2wQVR3AjJBUGtBEbQxkO0esCC8/jAiNUXOrxuNaDU2BpR5D/r z/X73R3BTichUU8KzjUqHZ7CjFjs1DHNgSrcGbNicUcZzsECJkgeALWDd8zdxQppVCQV S0fjFb4llUSLT2NvTAgGnzhzaMAEfCMyY9p0Hq/YygkMrO/ogB0iajoeqt+kgtc6R1bR /n0Cj+EpPzCTyLf+KQFV/K2HiefP2MZms1mpzXOxIxl/7a/YuoBXKqD5kNjpJ24fxLcq xxjnsbnP2eMTrwaEiYnrpOg9CELoOtu/MQ7z7p6JeQdl7qIeEmy/oK5GLr8CSQaslVmB 2Xsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=xsuXY8vhSMEbWJGhREr+fl4UvWb8qIXSRM77CwkJJsQ=; b=kYmLaDN7Rm0NxQYFn6h84X7BO5Dj6TRFgcJIZIaXSGOig1d17osr/1eCdZFoyeavcJ qLyXOgC9k/ZeW2cc53Saw/xtx+OS6eif65mjYKpVblZ7fcNk6MV3Tz0hGUJsvjlt67s2 PwIaNAttvxLp/sCzP4t5X7PqxJCop1GLZ0zEd+H3dHmF7M4YtJ8HniQdU3t+G69ElYHF kbtRPHmrXlM6ImNdSkmRcB+FCseGYXPQSfqhKsEnJDRxuiNRR6SfrRzfKB8z2RKNzbsM DKKi3ZMW0xYjDI7N7WWco0UEtTyx56n+y3beZFDuIINuE5E9uShJXfRgJ6HFI48AB284 A/Lg== X-Gm-Message-State: ABUngvclz/yHidfmPIwxQjokpkuCDRQeh89lhBUupAS+gWybUrdtqmpPI6fJLxP7moB6Bg== X-Received: by 10.36.37.199 with SMTP id g190mr1306701itg.66.1479357153899; Wed, 16 Nov 2016 20:32:33 -0800 (PST) Received: from zony ([45.2.7.65]) by smtp.googlemail.com with ESMTPSA id p20sm5218834itc.2.2016.11.16.20.32.33 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 16 Nov 2016 20:32:33 -0800 (PST) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> Date: Wed, 16 Nov 2016 23:33:23 -0500 In-Reply-To: <878tsihhhl.fsf@secretsauce.net> (Dima Kogan's message of "Wed, 16 Nov 2016 20:15:34 -0800") Message-ID: <87lgwilo8c.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.5 (/) Dima Kogan writes: > New patch attached. > [...] > + > + ;; I advance to the next hunk with skip-hunk-start set to t > + ;; because I want the behavior of moving to the next logical > + ;; hunk, not the legacy behavior where were would sometimes stay > + ;; on the curent hunk. This is the behavior we get when > + ;; navigating through hunks interactively, and we want it when > + ;; applying hunks too. See http://debbugs.gnu.org/17544 > (when diff-advance-after-apply-hunk > - (diff-hunk-next)))))) > + (diff-hunk-next nil t)))))) Can you mention somewhere about avoiding an infinite loop that you were talking about before? (that's what I meant when I said to update this comment, but if it actually makes more sense to mention that somewhere else, please do so) Is it really a "legacy" behavior (considering that we *need* the "legacy" behavior in order to function correctly)? Also, I believe usual comment style is to use "we" not "I", and you didn't end the last sentence with a period. From debbugs-submit-bounces@debbugs.gnu.org Thu Nov 17 03:05:42 2016 Received: (at 17544) by debbugs.gnu.org; 17 Nov 2016 08:05:42 +0000 Received: from localhost ([127.0.0.1]:60100 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c7HhV-0000M1-LD for submit@debbugs.gnu.org; Thu, 17 Nov 2016 03:05:42 -0500 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:47061) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c7HhU-0000Ls-Ia for 17544@debbugs.gnu.org; Thu, 17 Nov 2016 03:05:41 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 5E70B20776; Thu, 17 Nov 2016 03:05:40 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Thu, 17 Nov 2016 03:05:40 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=PzEWjQtahnvPKWEMjvffd8R7xRE=; b=vKQNjQ pGP0LUAObhsyyUvhR3uIHQyA5zi71f0PU2/z9ApQ0g2+VUYkRfrkKtzxO0NVjwIE TnNI8ZMBOebUHVSpUvH5oNIMjOa2ENEp3lQI74Ni6iTF8yZluIsiLdH8VG9fTRZe t0cFlI7f/XmG/UhAqf/VjmG8sR0i9C4cVXatY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=smtpout; bh=PzEWjQtahnvPKW EMjvffd8R7xRE=; b=LWTJ5Fz69UlcZNTPjt7eWEh1FlJxxS7abG2FrxAW1gQNEQ 4/7GQoGs6VUgAqohG5PqyGXmbSUz+Pevy4q6sWQpLAoCcBXB+wYFargIR/OUhV7N 1vSofgTfzq2VTGDwBEb99iMjwoqUz/e/NpvR2nNq8AdlOc+GjwvwGqmvv9lw4= X-ME-Sender: X-Sasl-enc: 18/D5mlwoBpbJgfJckeJKsomnvaTWZ9x98b383qnNBZ8 1479369939 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id D74CD24426; Thu, 17 Nov 2016 03:05:39 -0500 (EST) Received: from dima by scrawny with local (Exim 4.87) (envelope-from ) id 1c7HhS-0005KC-IY; Thu, 17 Nov 2016 00:05:38 -0800 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> <87lgwilo8c.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 26.0.50.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87lgwilo8c.fsf@users.sourceforge.net> Date: Thu, 17 Nov 2016 00:05:38 -0800 Message-ID: <871syah6p9.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain New patch attached. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From 769915004c4eb523d9e39da7ac96ec2174a213f9 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 130 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 117 insertions(+), 13 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..2a4c3d9 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ diff--auto-refine-data ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,101 @@ diff--auto-refine-data (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (skip-hunk-start + what orig + header-re goto-start-func count) + "I override the default diff-{hunk,file}-{next,prev} +implementation by skipping any lines that are associated with +this hunk/file but precede the hunk-start marker. For instance, a +diff file could contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Here I move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker" + (if (not skip-hunk-start) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; I trap the error + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +;; These functions all take a skip-hunk-start argument which controls +;; whether we skip pre-hunk-start text or not. In interactive uses we +;; always want to do this, but the simple behavior is still necessary +;; to, for example, avoid an infinite loop: +;; +;; diff-hunk-next calls +;; diff--wrap-navigation calls +;; diff-bounds-of-hunk calls +;; diff-beginning-of-hunk calls +;; diff-hunk-next +;; +;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the +;; inner one does not, which breaks the loop +(defun diff-hunk-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th hunk" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(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)) + +(defun diff-file-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th file" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count skip-hunk-start) + "Go to the next COUNT'th file" + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +675,13 @@ diff-bounds-of-hunk (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1760,9 @@ diff-find-source-location SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1771,7 @@ diff-find-source-location ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1783,8 +1879,15 @@ diff-apply-hunk ;; Display BUF in a window (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) + + ;; We advance to the next hunk with skip-hunk-start set to t + ;; because we want the behavior of moving to the next logical + ;; hunk, not the original behavior where were would sometimes + ;; stay on the curent hunk. This is the behavior we get when + ;; navigating through hunks interactively, and we want it when + ;; applying hunks too. See http://debbugs.gnu.org/17544 (when diff-advance-after-apply-hunk - (diff-hunk-next)))))) + (diff-hunk-next nil t)))))) (defun diff-test-hunk (&optional reverse) @@ -1865,14 +1968,15 @@ diff-current-defun (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2063,8 @@ diff-refine-hunk (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2072,7 @@ diff-refine-hunk (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.8.0.rc3 --=-=-= Content-Type: text/plain npostavs@users.sourceforge.net writes: > Can you mention somewhere about avoiding an infinite loop that you were > talking about before? (that's what I meant when I said to update this > comment, but if it actually makes more sense to mention that somewhere > else, please do so) > Is it really a "legacy" behavior (considering that we *need* the "legacy" > behavior in order to function correctly)? Added comment, changed nomenclature. > Also, I believe usual comment style is to use "we" not "I", and you > didn't end the last sentence with a period. Changed the I/we. I'm omitting the trailing . on purpose to make sure it doesn't get confused with the URL. --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sat Nov 19 21:36:37 2016 Received: (at 17544) by debbugs.gnu.org; 20 Nov 2016 02:36:37 +0000 Received: from localhost ([127.0.0.1]:35458 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c8Hzh-0006Ig-El for submit@debbugs.gnu.org; Sat, 19 Nov 2016 21:36:37 -0500 Received: from mail-io0-f194.google.com ([209.85.223.194]:33895) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c8Hzf-0006IQ-V7 for 17544@debbugs.gnu.org; Sat, 19 Nov 2016 21:36:36 -0500 Received: by mail-io0-f194.google.com with SMTP id n13so2515444ioe.1 for <17544@debbugs.gnu.org>; Sat, 19 Nov 2016 18:36:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=AZC9S0TiSqmBat9Pk3g1vU6ZH1g+vzUuS/2/dhoZny4=; b=QD6MiNFl47tcUrM2htL7t/XgYqAUnRM5RHjJB5Th+O2LPeyKTKXb4TuadX4qhgL53/ bCUm2Fw9hcg7SKrAAG1nrhnIQKbeQdm/1ZhHy8pwtvC04ZRqh4NQp38fXWs06aw0e+ty 0k6m5hiLHWlfb/JfzUDVyxZFMgDhDZuAmwp05zmpwgksYfAC4AE2YT1drhbiBEqo8Cbw CdNEYValgoUUhPt5Ane/VYcW14fCbT+MS5bSk8YWQmmdAL01v/TndyJDpC6uIMUD7Rf1 F8YKCwd4Bsmc8rw/W7T1mIHwpYj46Fe/WlUsS3apy3IDFJWRKgqKAdybgkPL80T1WbxV OKlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=AZC9S0TiSqmBat9Pk3g1vU6ZH1g+vzUuS/2/dhoZny4=; b=k2uUexcIyh5dlrLkgqJKnB8wv08IymAujTyewDySsWfv6bVhZBt8MbRVz6k7pHa1ia OJiw8dWa4DoYfGNzPDMVApmk6Lh4LBu3sKTH6YRxRB9Sm8pTApd5YLuP1BtMlWI9JaIV rcLA/vhvj0UtTlu+I7GowsDL+KFpkFG+POWL6X6j18zECfz0lMvfcSFLerj5r48EK+Nr v0Wx/ii8swWRvn0XzphDunlIw4975SSWU0XHlGTkvqEYnvi/aY96pvMuYEup1V/OfhkU 5Nqzgy7zURJEpCQU89Um8xpUx2NRZsfe8y7XUEPFKz6S60pY5Z7F9umdGbSsBsYzhT32 KhqQ== X-Gm-Message-State: AKaTC02kuQm5cfBtZRmzzzQuvnNzr5HWPPlyAh4PiV0SXCMl136q0ut7ycFjLZJkorYdPg== X-Received: by 10.107.7.27 with SMTP id 27mr5283187ioh.130.1479609390362; Sat, 19 Nov 2016 18:36:30 -0800 (PST) Received: from zony ([45.2.7.65]) by smtp.googlemail.com with ESMTPSA id 197sm5749825iou.42.2016.11.19.18.36.28 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 19 Nov 2016 18:36:29 -0800 (PST) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> <87lgwilo8c.fsf@users.sourceforge.net> <871syah6p9.fsf@secretsauce.net> Date: Sat, 19 Nov 2016 21:37:20 -0500 In-Reply-To: <871syah6p9.fsf@secretsauce.net> (Dima Kogan's message of "Thu, 17 Nov 2016 00:05:38 -0800") Message-ID: <87zikukhb3.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.5 (/) Dima Kogan writes: > +(defun diff--wrap-navigation (skip-hunk-start > + what orig > + header-re goto-start-func count) > + "I override the default diff-{hunk,file}-{next,prev} > +implementation by skipping any lines that are associated with The first line of a docstring should be a single sentence. > +this hunk/file but precede the hunk-start marker. For instance, a > +If a point is on 'index', then the point is considered to be in > +this first hunk. Here I move the point to the @@... marker before Double space end of sentence. No need to mention "I" here, it should be phrased imperatively, as in "Override the default...", and "Move the point...". > +executing the default diff-hunk-next/prev implementation to move > +to the NEXT marker" End sentences with a period. > + ;; I trap the error End sentences with a period, no need to mention "I". > +;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the > +;; inner one does not, which breaks the loop > +(defun diff-hunk-prev (&optional count skip-hunk-start) > + "Go to the previous COUNT'th hunk" > +(defun diff-hunk-next (&optional count skip-hunk-start) > + "Go to the next COUNT'th hunk" > +(defun diff-file-prev (&optional count skip-hunk-start) > + "Go to the previous COUNT'th file" > +(defun diff-file-next (&optional count skip-hunk-start) > + "Go to the next COUNT'th file" > + ;; There's no next hunk, so just take the one we have End sentences with a period. > + (t (list beg end)))))) > + ;; applying hunks too. See http://debbugs.gnu.org/17544 >> Also, I believe usual comment style is to use "we" not "I", and you >> didn't end the last sentence with a period. > > Changed the I/we. I'm omitting the trailing . on purpose to make sure it > doesn't get confused with the URL. I suggest ";; applying hunks too (see http://debbugs.gnu.org/17544).", referencing the bug with "Bug #17544" instead of a full URL could also work. From debbugs-submit-bounces@debbugs.gnu.org Mon Nov 21 02:23:08 2016 Received: (at 17544) by debbugs.gnu.org; 21 Nov 2016 07:23:09 +0000 Received: from localhost ([127.0.0.1]:36645 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c8iwW-0000FB-Ei for submit@debbugs.gnu.org; Mon, 21 Nov 2016 02:23:08 -0500 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:33369) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c8iwU-0000F1-L7 for 17544@debbugs.gnu.org; Mon, 21 Nov 2016 02:23:07 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 13F3A206D2; Mon, 21 Nov 2016 02:23:06 -0500 (EST) Received: from frontend1 ([10.202.2.160]) by compute1.internal (MEProxy); Mon, 21 Nov 2016 02:23:06 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=DRbNj6dSgL0JOoknHBbG0tfnIy8=; b=JvtEQj ZTMDdCXvKkx3qGW1IfZeTTJoqkAR5DNa6HpC4OZT1kWV5vQXMAalEX6HvtbWc3GP dI6h16LyjTu7XOE+NSd/9xSrhgkiaFNSOXCxvMabsrXXB1q7ZhhERBo4tnXF6rVg txqATTJpPdDGwOE+sYGE0+lcR0gptBNZsnBw4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=smtpout; bh=DRbNj6dSgL0JOo knHBbG0tfnIy8=; b=pfJpIDM/XULWidyYYsX0zTDBLOn03PhB68JnBsn2MJeSvR hnhuJ3o6a7sLDBCZBl/GLBM1jDvUSjsXFtUkdCKUXp8c0//HjXlRb9r0kKSgjEGR 03hzcQvMsW/ocmwMyPi/vHjfiPmH0P3Ei0syCTuGXnERchUPFjtoL61BBB2lA= X-ME-Sender: X-Sasl-enc: xGlUAJA2u3bmt5X6yxdfoFNoXETH/56Bl4dh0ed3ZamD 1479712985 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 947987E070; Mon, 21 Nov 2016 02:23:05 -0500 (EST) Received: from dima by scrawny with local (Exim 4.87) (envelope-from ) id 1c8iwS-0005XV-5g; Sun, 20 Nov 2016 23:23:04 -0800 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> <87lgwilo8c.fsf@users.sourceforge.net> <871syah6p9.fsf@secretsauce.net> <87zikukhb3.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 26.0.50.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87zikukhb3.fsf@users.sourceforge.net> Date: Sun, 20 Nov 2016 23:23:04 -0800 Message-ID: <87r365fg9z.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain npostavs@users.sourceforge.net writes: > The first line of a docstring should be a single sentence. > > Double space end of sentence. No need to mention "I" here, it should be > phrased imperatively, as in "Override the default...", and "Move the point...". > > End sentences with a period. > > I suggest ";; applying hunks too (see http://debbugs.gnu.org/17544).", > referencing the bug with "Bug #17544" instead of a full URL could also > work. Attached. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improved-diff-mode-navigation-manipulation.patch >From d954bf87907ed2e3f94347c2249b305c7e231832 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improved diff-mode navigation/manipulation Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. --- lisp/vc/diff-mode.el | 131 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 118 insertions(+), 13 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..c9a5f89 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ diff--auto-refine-data ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,102 @@ diff--auto-refine-data (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (skip-hunk-start + what orig + header-re goto-start-func count) + "Wrap diff-{hunk,file}-{next,prev} for more intuitive behavior. +Override the default diff-{hunk,file}-{next,prev} implementation +by skipping any lines that are associated with this hunk/file but +precede the hunk-start marker. For instance, a diff file could +contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker." + (if (not skip-hunk-start) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; Trap the error. + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +;; These functions all take a skip-hunk-start argument which controls +;; whether we skip pre-hunk-start text or not. In interactive uses we +;; always want to do this, but the simple behavior is still necessary +;; to, for example, avoid an infinite loop: +;; +;; diff-hunk-next calls +;; diff--wrap-navigation calls +;; diff-bounds-of-hunk calls +;; diff-beginning-of-hunk calls +;; diff-hunk-next +;; +;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the +;; inner one does not, which breaks the loop. +(defun diff-hunk-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th hunk." + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(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)) + +(defun diff-file-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th file." + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count skip-hunk-start) + "Go to the next COUNT'th file." + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +676,13 @@ diff-bounds-of-hunk (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have. + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1761,9 @@ diff-find-source-location SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1772,7 @@ diff-find-source-location ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1783,8 +1880,15 @@ diff-apply-hunk ;; Display BUF in a window (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) + + ;; Advance to the next hunk with skip-hunk-start set to t + ;; because we want the behavior of moving to the next logical + ;; hunk, not the original behavior where were would sometimes + ;; stay on the curent hunk. This is the behavior we get when + ;; navigating through hunks interactively, and we want it when + ;; applying hunks too (see http://debbugs.gnu.org/17544). (when diff-advance-after-apply-hunk - (diff-hunk-next)))))) + (diff-hunk-next nil t)))))) (defun diff-test-hunk (&optional reverse) @@ -1865,14 +1969,15 @@ diff-current-defun (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2064,8 @@ diff-refine-hunk (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2073,7 @@ diff-refine-hunk (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.8.0.rc3 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 22 19:41:19 2016 Received: (at 17544) by debbugs.gnu.org; 23 Nov 2016 00:41:19 +0000 Received: from localhost ([127.0.0.1]:38740 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c9Lck-00075p-UD for submit@debbugs.gnu.org; Tue, 22 Nov 2016 19:41:19 -0500 Received: from mail-io0-f195.google.com ([209.85.223.195]:35419) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c9Lcj-00075W-65 for 17544@debbugs.gnu.org; Tue, 22 Nov 2016 19:41:17 -0500 Received: by mail-io0-f195.google.com with SMTP id h133so12975673ioe.2 for <17544@debbugs.gnu.org>; Tue, 22 Nov 2016 16:41:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=0JC4MlBvWOJp4daJtCsGfvKSAS0P2Lj1mE6NsoB09Do=; b=kn1pxlPL9A7KCbuVPBiEiBnBngh4WEnoS76yuTzYPptXaRlYmS26jgTu3Q34gSav9Q RvQZ2OjGda3cclfc2T0z4PqyCZJonFY7YlQesdU4HPf0i2ye+Dv7J6W0QUg0+KbDfHcs DyxF6IL69tlhXGf8nKKCo3q89ZZHHxcsx91s1DW3NLT2+vvTeC+seuDLA1wheMPqxZjG 3cA6He31xQFm/QjHgo8gq/trYig8esPskTWVaZ1jFUUxgOOlUFocRSl/iMvoAaTUtQMY wCbXtspm3bq1/GBKiBKuoe77/OSEDfb8XC4KcZiEwIOqiXitYcpJtj18AnAqz/WqEtpP ZzCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=0JC4MlBvWOJp4daJtCsGfvKSAS0P2Lj1mE6NsoB09Do=; b=GnZXi72ASWNSGReGE2sU3SZh8iTVCaXFvIHrCW8MHktt4R5xNmXWa8PgKEpbLY8scQ S5QtkzVwllnB7kQwdCYryHYmYx3rxa+JLBLrr1TzcbIxPCdo6Q0xcfCZcF9vohsaaUrD PdMQT0vQ9eUIp1BKoJdm12BH1hHlpKFXtltDxORk3L4EOG/Yh42tzgTMl6D/TinsRU21 0flY6uWSb1r5u00y+g+tXLXfpUD5X45M9m0Vq2swYXC+GCkcTa8EZDar6lmHQEulpqYG aM33Z4oahe0SBb24VboT2bZFdCDqyUzxbBOKbKLnCLs91fxb4VbD1Tfx7rjCLgvDeSnf FcSA== X-Gm-Message-State: AKaTC01UiVsCMJGZXuGFEeleuSVXKUK1n31L+NP4i3aTYaCmMl6qnhSDpwoEd59ve5DfVA== X-Received: by 10.107.34.74 with SMTP id i71mr1075478ioi.24.1479861671448; Tue, 22 Nov 2016 16:41:11 -0800 (PST) Received: from zony ([45.2.7.65]) by smtp.googlemail.com with ESMTPSA id i18sm2450073itb.0.2016.11.22.16.41.09 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 22 Nov 2016 16:41:10 -0800 (PST) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> <87lgwilo8c.fsf@users.sourceforge.net> <871syah6p9.fsf@secretsauce.net> <87zikukhb3.fsf@users.sourceforge.net> <87r365fg9z.fsf@secretsauce.net> Date: Tue, 22 Nov 2016 19:42:03 -0500 In-Reply-To: <87r365fg9z.fsf@secretsauce.net> (Dima Kogan's message of "Sun, 20 Nov 2016 23:23:04 -0800") Message-ID: <87d1hnjack.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.5 (/) > From: Dima Kogan > Date: Wed, 14 Sep 2016 15:25:06 -0700 > Subject: [PATCH] Improved diff-mode navigation/manipulation The commit message should also have double spaced sentences and should end with a ChangeLog style entry. Apart from that I think is ready to push to master. From debbugs-submit-bounces@debbugs.gnu.org Wed Nov 23 16:11:28 2016 Received: (at 17544) by debbugs.gnu.org; 23 Nov 2016 21:11:28 +0000 Received: from localhost ([127.0.0.1]:39636 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c9epD-0006W3-J0 for submit@debbugs.gnu.org; Wed, 23 Nov 2016 16:11:28 -0500 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:53979) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1c9epC-0006Vw-6w for 17544@debbugs.gnu.org; Wed, 23 Nov 2016 16:11:26 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id B35E421B9C; Wed, 23 Nov 2016 16:11:25 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Wed, 23 Nov 2016 16:11:25 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=cc:content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=TPRpmUoogmwkiGH5Ad2SQeCzeyk=; b=Ea/SUp hAjlXf6vVvjhGsu9j4KuQo7xAMjH4U5vmNYQAIvDajPHUmEfGj+KVVPcaAu3hGIK DXimsaGC2cEgQYy5c3WZ5P69rvmMVkEMTy2S4iAFGohLZBCJCCCiRT9BgswoWo+b rDzlxO9mfWDpu7zz080ZuodSrA/EjLHqHrYJQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc:x-sasl-enc; s=smtpout; bh=TPRpmUoogmwkiG H5Ad2SQeCzeyk=; b=W9ntuHg7ehtM3tmx6DJasmgIjvsSxS3SXIGhuCbCB+ngqi PQUyvysCgjeofBAuuGy4OdILn35u1uQwj99sow/Uuxw5m1SNhAl7BRQEGziFIAc7 FklaHnvPDyaVf6HV9RGK7ENm3e65iakK1uilonNC1PT+RmM+UWN7EQ+AfNnTE= X-ME-Sender: X-Sasl-enc: zoUoODE1XNECUBVEDHcgo40SB4swNdiLys3u1wOdr2ng 1479935485 Received: from scrawny (50-1-153-216.dsl.dynamic.fusionbroadband.com [50.1.153.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 38C1025056; Wed, 23 Nov 2016 16:11:25 -0500 (EST) Received: from [::1] (helo=scrawny) by scrawny with esmtp (Exim 4.87) (envelope-from ) id 1c9ep9-0004RR-UI; Wed, 23 Nov 2016 13:11:23 -0800 References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> <87lgwilo8c.fsf@users.sourceforge.net> <871syah6p9.fsf@secretsauce.net> <87zikukhb3.fsf@users.sourceforge.net> <87r365fg9z.fsf@secretsauce.net> <87d1hnjack.fsf@users.sourceforge.net> User-agent: mu4e 0.9.17; emacs 26.0.50.1 From: Dima Kogan To: npostavs@users.sourceforge.net Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation In-reply-to: <87d1hnjack.fsf@users.sourceforge.net> Date: Wed, 23 Nov 2016 13:11:23 -0800 Message-ID: <87lgw96gw4.fsf@secretsauce.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) --=-=-= Content-Type: text/plain npostavs@users.sourceforge.net writes: > The commit message should also have double spaced sentences and should > end with a ChangeLog style entry. Apart from that I think is ready to > push to master. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Improve-diff-mode-navigation-manipulation.patch >From d3950fca6cb1f3aa097cec0f524e38b9a7f05303 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Wed, 14 Sep 2016 15:25:06 -0700 Subject: [PATCH] Improve diff-mode navigation/manipulation This is Bug #17544. Navigation and use of diff buffers had several annoying corner cases that this patch fixes. These corner cases were largely due to inconsistent treatment of file headers. Say you have a diff such as this: --- aaa +++ bbb @@ -52,7 +52,7 @@ hunk1 @@ -74,7 +74,7 @@ hunk2 --- ccc +++ ddd @@ -608,6 +608,6 @@ hunk3 @@ -654,7 +654,7 @@ hunk4 The file headers here are the '---' and '+++' lines. With the point on such a line, hunk operations would sometimes refer to the next hunk and sometimes to the previous hunk. Most of the time it would be the previous hunk, which is not what the user would expect. This patch consistently treats such headers as the next hunk. So with this patch, if the point is on the '--- ccc' line, the point is seen as referring to hunk3. Specific behaviors this fixes are: 1. It should be possible to place the point in the middle of a diff buffer, and press M-k repeatedly to kill hunks in the order they appear in the buffer. With the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch. 2. Similarly, it should be possible to apply hunks in order. Previously with the point at the start, C-c C-a would apply the hunk1, then move the point to the first @@ header, and thus C-c C-a would try to apply the same hunk again. lisp/vc/diff-mode.el (diff--wrap-navigation): New function to add better navigation logic to diff-{hunk,file}-{next,prev}. lisp/vc/diff-mode.el (diff-hunk-next, diff-hunk-prev, diff-file-next, diff-file-prev): Better navigation logic if skip-hunk-start is true, which happens when called interactively. lisp/vc/diff-mode.el (diff-bounds-of-hunk, diff-find-source-location, diff-apply-hunk, diff-current-defun, diff-refine-hunk): Small tweaks to improve hunk navigation. --- lisp/vc/diff-mode.el | 131 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 118 insertions(+), 13 deletions(-) diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el index 58498cb..c9a5f89 100644 --- a/lisp/vc/diff-mode.el +++ b/lisp/vc/diff-mode.el @@ -551,7 +551,7 @@ diff--auto-refine-data ;; Define diff-{hunk,file}-{prev,next} (easy-mmode-define-navigation - diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view + diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view (when diff-auto-refine-mode (unless (prog1 diff--auto-refine-data (setq diff--auto-refine-data @@ -570,7 +570,102 @@ diff--auto-refine-data (diff-refine-hunk)))))))))))) (easy-mmode-define-navigation - diff-file diff-file-header-re "file" diff-end-of-file) + diff--internal-file diff-file-header-re "file" diff-end-of-file) + +(defun diff--wrap-navigation (skip-hunk-start + what orig + header-re goto-start-func count) + "Wrap diff-{hunk,file}-{next,prev} for more intuitive behavior. +Override the default diff-{hunk,file}-{next,prev} implementation +by skipping any lines that are associated with this hunk/file but +precede the hunk-start marker. For instance, a diff file could +contain + +diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el +index 923de9a..6b1c24f 100644 +--- a/lisp/vc/diff-mode.el ++++ b/lisp/vc/diff-mode.el +@@ -590,6 +590,22 @@ +....... + +If a point is on 'index', then the point is considered to be in +this first hunk. Move the point to the @@... marker before +executing the default diff-hunk-next/prev implementation to move +to the NEXT marker." + (if (not skip-hunk-start) + (funcall orig count) + + (let ((start (point))) + (funcall goto-start-func) + + ;; Trap the error. + (condition-case nil + (funcall orig count) + (error nil)) + + (when (not (looking-at header-re)) + (goto-char start) + (user-error (format "No %s" what)))))) + +;; These functions all take a skip-hunk-start argument which controls +;; whether we skip pre-hunk-start text or not. In interactive uses we +;; always want to do this, but the simple behavior is still necessary +;; to, for example, avoid an infinite loop: +;; +;; diff-hunk-next calls +;; diff--wrap-navigation calls +;; diff-bounds-of-hunk calls +;; diff-beginning-of-hunk calls +;; diff-hunk-next +;; +;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the +;; inner one does not, which breaks the loop. +(defun diff-hunk-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th hunk." + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev hunk" + 'diff--internal-hunk-prev + diff-hunk-header-re + (lambda () (goto-char (car (diff-bounds-of-hunk)))) + count)) + +(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)) + +(defun diff-file-prev (&optional count skip-hunk-start) + "Go to the previous COUNT'th file." + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "prev file" + 'diff--internal-file-prev + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + +(defun diff-file-next (&optional count skip-hunk-start) + "Go to the next COUNT'th file." + (interactive (list (prefix-numeric-value current-prefix-arg) t)) + (diff--wrap-navigation + skip-hunk-start + "next file" + 'diff--internal-file-next + diff-file-header-re + (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next)) + count)) + + + (defun diff-bounds-of-hunk () "Return the bounds of the diff hunk at point. @@ -581,12 +676,13 @@ diff-bounds-of-hunk (let ((pos (point)) (beg (diff-beginning-of-hunk t)) (end (diff-end-of-hunk))) - (cond ((>= end pos) + (cond ((> end pos) (list beg end)) ;; If this hunk ends above POS, consider the next hunk. ((re-search-forward diff-hunk-header-re nil t) (list (match-beginning 0) (diff-end-of-hunk))) - (t (error "No hunk found")))))) + ;; There's no next hunk, so just take the one we have. + (t (list beg end)))))) (defun diff-bounds-of-file () "Return the bounds of the file segment at point. @@ -1665,8 +1761,9 @@ diff-find-source-location SWITCHED is non-nil if the patch is already applied. NOPROMPT, if non-nil, means not to prompt the user." (save-excursion - (let* ((other (diff-xor other-file diff-jump-to-old-file)) - (char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (other (diff-xor other-file diff-jump-to-old-file)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) ;; Check that the hunk is well-formed. Otherwise diff-mode and ;; the user may disagree on what constitutes the hunk ;; (e.g. because an empty line truncates the hunk mid-course), @@ -1675,7 +1772,7 @@ diff-find-source-location ;; Suppress check when NOPROMPT is non-nil (Bug#3033). (_ (unless noprompt (diff-sanity-check-hunk))) (hunk (buffer-substring - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (old (diff-hunk-text hunk reverse char-offset)) (new (diff-hunk-text hunk (not reverse) char-offset)) ;; Find the location specification. @@ -1783,8 +1880,15 @@ diff-apply-hunk ;; Display BUF in a window (set-window-point (display-buffer buf) (+ (car pos) (cdr new))) (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil) + + ;; Advance to the next hunk with skip-hunk-start set to t + ;; because we want the behavior of moving to the next logical + ;; hunk, not the original behavior where were would sometimes + ;; stay on the curent hunk. This is the behavior we get when + ;; navigating through hunks interactively, and we want it when + ;; applying hunks too (see http://debbugs.gnu.org/17544). (when diff-advance-after-apply-hunk - (diff-hunk-next)))))) + (diff-hunk-next nil t)))))) (defun diff-test-hunk (&optional reverse) @@ -1865,14 +1969,15 @@ diff-current-defun (defun diff-ignore-whitespace-hunk () "Re-diff the current hunk, ignoring whitespace differences." (interactive) - (let* ((char-offset (- (point) (diff-beginning-of-hunk t))) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (char-offset (- (point) (goto-char (car hunk-bounds)))) (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b"))) (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)") (error "Can't find line number")) (string-to-number (match-string 1)))) (inhibit-read-only t) (hunk (delete-and-extract-region - (point) (save-excursion (diff-end-of-hunk) (point)))) + (point) (cadr hunk-bounds))) (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1. (file1 (make-temp-file "diff1")) (file2 (make-temp-file "diff2")) @@ -1959,8 +2064,8 @@ diff-refine-hunk (interactive) (require 'smerge-mode) (save-excursion - (diff-beginning-of-hunk t) - (let* ((start (point)) + (let* ((hunk-bounds (diff-bounds-of-hunk)) + (start (goto-char (car hunk-bounds))) (style (diff-hunk-style)) ;Skips the hunk header as well. (beg (point)) (props-c '((diff-mode . fine) (face diff-refine-changed))) @@ -1968,7 +2073,7 @@ diff-refine-hunk (props-a '((diff-mode . fine) (face diff-refine-added))) ;; Be careful to go back to `start' so diff-end-of-hunk gets ;; to read the hunk header's line info. - (end (progn (goto-char start) (diff-end-of-hunk) (point)))) + (end (goto-char (cadr hunk-bounds)))) (remove-overlays beg end 'diff-mode 'fine) -- 2.8.0.rc3 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Mon Nov 28 23:09:16 2016 Received: (at 17544) by debbugs.gnu.org; 29 Nov 2016 04:09:16 +0000 Received: from localhost ([127.0.0.1]:45464 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cBZjI-0006W8-J3 for submit@debbugs.gnu.org; Mon, 28 Nov 2016 23:09:16 -0500 Received: from mail-io0-f194.google.com ([209.85.223.194]:36717) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cBZjH-0006Vq-Ak; Mon, 28 Nov 2016 23:09:15 -0500 Received: by mail-io0-f194.google.com with SMTP id k19so26938923iod.3; Mon, 28 Nov 2016 20:09:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=sO6O8J6soYQT/nfiWDt3uoEk4UI8VIN9JTJL4vBzESQ=; b=zO94U6G5SgQj49wGjaLumccLtBOudcyoRD2E1yI7m59HBweSqY1145Z2xj6YTppPY1 5SUrMID6ssqPsHEmm18+eNVt2iLXKlV+72GlkTM8FK9Fxcc/rKuaR9gzGU9eA4wQsgrd +MXmzIsHnesq0lb+QcXhu1z5rjjaydmhmO5vR4BQz1wkCrqNyI82chQU0yUlkEOKcPcU 0juMWZ1vFQ2+i6deIAHoy8sG2uaec5s0UiGmpE4I1OqiCiTbUrG2/0WiQWUzKUKPlSXI BvHiAHDe7lXTD9E4topN9elO/VN8utHTd0g2FnvaURGbk8sfcO3JKrEtYqII/ffFwtvY GY6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=sO6O8J6soYQT/nfiWDt3uoEk4UI8VIN9JTJL4vBzESQ=; b=Eawlh2Kt5iZlndjwn8AoV9k4tClhwEcSK7YpqqvY2qg6wr38WkLvzVIZy+9SPaVuwL Vpgz/pNZbQxrxEuhU+6reszjhOYmqzYCGFzjupEpAfJt22gfR+Crn1mOvhf6o7Z6tny6 tdH6p0zQ33rvvyKGAXiCy+k2tUGErsgIyic3/kNY8VFEQK8QaFL/6+t1vkPgmYs/SSYB DD8ai8zp/zqUP5C5qsZKHZaMs608JQLUHmYeX+44aqE1B9/zijyATd4nX8YjT82+o2AJ CCPDeN4S+zudUsuwXYKcQ7Dw5tmKaSkxW6VGZwsg2gPhSnntOt852GNNwMUKYFMvyLdU FB8w== X-Gm-Message-State: AKaTC011YuBn3fwXJVA8TqfhgdHIN3utzaVi39Ek4iRxPO1DHbdAg0ZX3kU1rivqcr5nPQ== X-Received: by 10.107.36.144 with SMTP id k138mr21871937iok.177.1480392549580; Mon, 28 Nov 2016 20:09:09 -0800 (PST) Received: from zony ([45.2.7.65]) by smtp.googlemail.com with ESMTPSA id j189sm21064123ioe.14.2016.11.28.20.09.08 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 28 Nov 2016 20:09:08 -0800 (PST) From: npostavs@users.sourceforge.net To: Dima Kogan Subject: Re: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation References: <87ha4jgw53.fsf@secretsauce.net> <87si0ion1c.fsf@gnus.org> <87k2et8hr4.fsf@secretsauce.net> <83a8fpe1dr.fsf@gnu.org> <87h99w8ynv.fsf@secretsauce.net> <87k2es9weh.fsf@users.sourceforge.net> <87oa4018r3.fsf@secretsauce.net> <87twdi6rm9.fsf@secretsauce.net> <87h997nkqp.fsf@secretsauce.net> <874m44tmge.fsf@users.sourceforge.net> <87h983rg9c.fsf@secretsauce.net> <87mvhvsrta.fsf@users.sourceforge.net> <87wpgge0kl.fsf@secretsauce.net> <87twb9l8qi.fsf@users.sourceforge.net> <878tsihhhl.fsf@secretsauce.net> <87lgwilo8c.fsf@users.sourceforge.net> <871syah6p9.fsf@secretsauce.net> <87zikukhb3.fsf@users.sourceforge.net> <87r365fg9z.fsf@secretsauce.net> <87d1hnjack.fsf@users.sourceforge.net> <87lgw96gw4.fsf@secretsauce.net> Date: Mon, 28 Nov 2016 23:10:05 -0500 In-Reply-To: <87lgw96gw4.fsf@secretsauce.net> (Dima Kogan's message of "Wed, 23 Nov 2016 13:11:23 -0800") Message-ID: <8760n7hqoy.fsf@users.sourceforge.net> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.5 (/) X-Debbugs-Envelope-To: 17544 Cc: Eli Zaretskii , Andreas Schwab , 17544@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: 0.5 (/) close 17544 26.1 quit Dima Kogan writes: > npostavs@users.sourceforge.net writes: > >> The commit message should also have double spaced sentences and should >> end with a ChangeLog style entry. Apart from that I think is ready to >> push to master. > >>>From d3950fca6cb1f3aa097cec0f524e38b9a7f05303 Mon Sep 17 00:00:00 2001 > From: Dima Kogan > Date: Wed, 14 Sep 2016 15:25:06 -0700 > Subject: [PATCH] Improve diff-mode navigation/manipulation Pushed as 2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085. [...] > > lisp/vc/diff-mode.el (diff--wrap-navigation): New function to add better > navigation logic to diff-{hunk,file}-{next,prev}. > > lisp/vc/diff-mode.el (diff-hunk-next, diff-hunk-prev, diff-file-next, > diff-file-prev): Better navigation logic if skip-hunk-start is true, > which happens when called interactively. > > lisp/vc/diff-mode.el (diff-bounds-of-hunk, diff-find-source-location, > diff-apply-hunk, diff-current-defun, diff-refine-hunk): Small tweaks to > improve hunk navigation. > --- I fixed the formatting of the ChangeLog entry. From unknown Mon Jun 23 07:48:02 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Tue, 27 Dec 2016 12:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator