From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Dima Kogan Newsgroups: gmane.emacs.bugs Subject: bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation Date: Wed, 14 Sep 2016 15:31:10 -0700 Message-ID: <87twdi6rm9.fsf@secretsauce.net> 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> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1473892355 12809 195.159.176.226 (14 Sep 2016 22:32:35 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 14 Sep 2016 22:32:35 +0000 (UTC) User-Agent: mu4e 0.9.17; emacs 25.1.1 Cc: Andreas Schwab , 17544@debbugs.gnu.org To: npostavs@users.sourceforge.net Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Sep 15 00:32:31 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bkIj6-0001dc-54 for geb-bug-gnu-emacs@m.gmane.org; Thu, 15 Sep 2016 00:32:20 +0200 Original-Received: from localhost ([::1]:59090 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkIj4-0000bK-3v for geb-bug-gnu-emacs@m.gmane.org; Wed, 14 Sep 2016 18:32:18 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:32772) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkIiv-0000bA-Vk for bug-gnu-emacs@gnu.org; Wed, 14 Sep 2016 18:32:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkIiq-0008DA-Sv for bug-gnu-emacs@gnu.org; Wed, 14 Sep 2016 18:32:08 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:33948) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkIio-0008CY-KB for bug-gnu-emacs@gnu.org; Wed, 14 Sep 2016 18:32:04 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bkIio-0002nP-DI for bug-gnu-emacs@gnu.org; Wed, 14 Sep 2016 18:32:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dima Kogan Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 14 Sep 2016 22:32:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 17544 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 17544-submit@debbugs.gnu.org id=B17544.147389227610695 (code B ref 17544); Wed, 14 Sep 2016 22:32:02 +0000 Original-Received: (at 17544) by debbugs.gnu.org; 14 Sep 2016 22:31:16 +0000 Original-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 Original-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 Original-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) Original-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 Original-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) Original-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 In-reply-to: <87oa4018r3.fsf@secretsauce.net> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:123315 Archived-At: --=-=-= 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 --=-=-=--