all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] Showing the relevant part of a diff
@ 2016-02-19  5:43 Herring, Davis
  2016-02-19  5:49 ` Lars Ingebrigtsen
  2016-02-19  8:47 ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Herring, Davis @ 2016-02-19  5:43 UTC (permalink / raw)
  To: emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

When looking at changes made to a file, the ones most likely to be interesting are those affecting the portion of the file in view.  The second attached patch causes `vc-diff' to place point at the place in the diff that corresponds to point in the file being compared (only if the current source is the endpoint of the diff, of course).

The first patch (starting from a recent master) is preparatory and provides (and uses) a defconst for the main work.

To resolve:
1. Would this need a NEWS entry?
2. Does it need to be customizable?  (M-< is all it takes to "recover the old behavior"...)
3. Is it worth supporting the (very rare) "normal" diff format?
4. Should M-x diff (or any other diff commands) behave similarly?
5. Do context/unified headers lack the (correct) length of the hunk often enough to bother with "donttrustheader"?  (This is the two FIXMEs in the patch.)

Davis

[-- Attachment #2: 0001-Clean-up-context.patch --]
[-- Type: application/octet-stream, Size: 5285 bytes --]

From 5aa3748d8e4d5cdb1fd911d8f098befcc28670fa Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Thu, 18 Feb 2016 07:47:45 -0700
Subject: [PATCH 1/2] Clean up context diff header regexps

* lisp/vc/diff-mode.el (diff-hunk-header-re-context): New const.
(diff-context-mid-hunk-header-re): Add missing ^.
(diff-hunk-header-re,diff-context->unified,diff-reverse-direction,
diff-sanity-check-hunk): Use it.
---
 lisp/vc/diff-mode.el |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index bada492..06d4502 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -379,8 +379,10 @@ well."
 
 (defconst diff-hunk-header-re-unified
   "^@@ -\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? \\+\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? @@")
+(defconst diff-hunk-header-re-context
+  "\\*\\{15\\}\\(?: .*\\)?\n\\*\\*\\* \\(\\([0-9]+\\)\\(?:,\\(-?[0-9]+\\)\\)?\\) \\*\\*\\*\\*")
 (defconst diff-context-mid-hunk-header-re
-  "--- \\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? ----$")
+  "^--- \\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? ----$")
 
 (defvar diff-use-changed-face (and (face-differs-from-default-p diff-changed-face)
 				   (not (face-equal diff-changed-face diff-added-face))
@@ -455,7 +457,9 @@ empty lines.  This makes the format less robust, but is tolerated.
 See http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01990.html")
 
 (defconst diff-hunk-header-re
-  (concat "^\\(?:" diff-hunk-header-re-unified ".*\\|\\*\\{15\\}.*\n\\*\\*\\* .+ \\*\\*\\*\\*\\|[0-9]+\\(,[0-9]+\\)?[acd][0-9]+\\(,[0-9]+\\)?\\)$"))
+  (concat "^\\(?:" diff-hunk-header-re-unified ".*\\|"
+          diff-hunk-header-re-context
+          "\\|[0-9]+\\(,[0-9]+\\)?[acd][0-9]+\\(,[0-9]+\\)?\\)$"))
 (defconst diff-file-header-re (concat "^\\(--- .+\n\\+\\+\\+ \\|\\*\\*\\* .+\n--- \\|[^-+!<>0-9@* \n]\\).+\n" (substring diff-hunk-header-re 1)))
 (defvar diff-narrowed-to nil)
 
@@ -1048,7 +1052,11 @@ With a prefix argument, convert unified format to context format."
           (inhibit-read-only t))
       (save-excursion
         (goto-char start)
-        (while (and (re-search-forward "^\\(\\(\\*\\*\\*\\) .+\n\\(---\\) .+\\|\\*\\{15\\}.*\n\\*\\*\\* \\([0-9]+\\),\\(-?[0-9]+\\) \\*\\*\\*\\*\\)\\(?: \\(.*\\)\\|$\\)" nil t)
+        (while (and (re-search-forward
+                     (concat "^\\(\\(\\*\\*\\*\\) .+\n\\(---\\) .+\\|"
+                             diff-hunk-header-re-context
+                             "\\)\\(?: \\(.*\\)\\|$\\)")
+                     nil t)
                     (< (point) end))
           (combine-after-change-calls
             (if (match-beginning 2)
@@ -1058,15 +1066,15 @@ With a prefix argument, convert unified format to context format."
                   (replace-match "+++" t t nil 3)
                   (replace-match "---" t t nil 2))
               ;; we matched a hunk header
-              (let ((line1s (match-string 4))
-                    (line1e (match-string 5))
+              (let ((line1s (match-string 5))
+                    (line1e (match-string 6))
                     (pt1 (match-beginning 0))
                     ;; Variables to use the special undo function.
                     (old-undo buffer-undo-list)
                     (old-end (marker-position end))
                     ;; We currently throw away the comment that can follow
                     ;; the hunk header.  FIXME: Preserve it instead!
-                    (reversible (not (match-end 6))))
+                    (reversible (not (match-end 7))))
                 (replace-match "")
                 (unless (re-search-forward
                          diff-context-mid-hunk-header-re nil t)
@@ -1144,7 +1152,11 @@ else cover the whole buffer."
 	(inhibit-read-only t))
     (save-excursion
       (goto-char start)
-      (while (and (re-search-forward "^\\(\\([-*][-*][-*] \\)\\(.+\\)\n\\([-+][-+][-+] \\)\\(.+\\)\\|\\*\\{15\\}.*\n\\*\\*\\* \\(.+\\) \\*\\*\\*\\*\\|@@ -\\([0-9,]+\\) \\+\\([0-9,]+\\) @@.*\\)$" nil t)
+      (while (and (re-search-forward
+                   (concat "^\\(\\([-*][-*][-*] \\)\\(.+\\)\n\\([-+][-+][-+] \\)\\(.+\\)\\|"
+                           diff-hunk-header-re-context
+                           "\\|@@ -\\([0-9,]+\\) \\+\\([0-9,]+\\) @@.*\\)$")
+                   nil t)
 		  (< (point) end))
 	(combine-after-change-calls
 	  (cond
@@ -1497,13 +1509,13 @@ Only works for unified diffs."
 
        ;; A context diff.
        ((eq (char-after) ?*)
-        (if (not (looking-at "\\*\\{15\\}\\(?: .*\\)?\n\\*\\*\\* \\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? \\*\\*\\*\\*"))
+        (if (not (looking-at diff-hunk-header-re-context))
             (error "Unrecognized context diff first hunk header format")
           (forward-line 2)
           (diff-sanity-check-context-hunk-half
-	   (if (match-end 2)
-	       (1+ (- (string-to-number (match-string 2))
-		      (string-to-number (match-string 1))))
+           (if (match-end 3)
+               (1+ (- (string-to-number (match-string 3))
+                      (string-to-number (match-string 2))))
 	     1))
           (if (not (looking-at diff-context-mid-hunk-header-re))
               (error "Unrecognized context diff second hunk header format")
-- 
1.6.4.4


[-- Attachment #3: 0002-Set-vc-diff-point.patch --]
[-- Type: application/octet-stream, Size: 6249 bytes --]

From 67b367ab757f013dca863b349758f9a757b3f7b9 Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Thu, 18 Feb 2016 08:10:51 -0700
Subject: [PATCH 2/2] Set vc-diff point to correspond to point in file

* lisp/vc/diff-mode.el (diff-goto-line): New function.
* lisp/vc/vc.el (vc-diff-finish): Use it.
(vc-diff-internal): Note location and pass it to `vc-diff-finish'.
---
 lisp/vc/diff-mode.el |   39 +++++++++++++++++++++++++++++++++++++++
 lisp/vc/vc.el        |   24 ++++++++++++++++++++----
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 06d4502..7ad3eee 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -596,6 +596,45 @@ 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-goto-line (file line column)
+  "Go to the place in this diff producing LINE in FILE.
+If LINE (in the new version of FILE) is included, move to it and then
+COLUMN characters forward.  If it is absent, go to the first hunk
+starting after LINE, or to the end if none does.
+If FILE isn't mentioned, go to the beginning of the buffer."
+  (goto-char (point-min))
+  (while (and (re-search-forward diff-file-header-re nil 'move)
+              (not (string-equal (diff-find-file-name) file))))
+  (if (eobp) (goto-char (point-min))
+    (forward-line -1)
+    (while
+        (progn
+          (condition-case nil (diff-hunk-next)
+            (error (goto-char (point-max))))
+          (cond ((eobp) nil)            ; end of the line
+                ((looking-at diff-hunk-header-re-unified)
+                 (let ((start (string-to-number (match-string 3)))
+                       ;; FIXME: assuming that we have the length
+                       (len (string-to-number (match-string 4))))
+                   (cond ((< line start) nil)    ; nothing found
+                         ((< line (+ start len)) ; this is our stop
+                          (dotimes (i (- line start -1))
+                            (while (progn (forward-line 1)
+                                          (eq (char-after) ?-))))
+                          (forward-char (1+ column)))
+                         (t))))         ; keep looking
+                ((looking-at diff-hunk-header-re-context)
+                 (re-search-forward diff-context-mid-hunk-header-re)
+                 (let ((start (string-to-number (match-string 2)))
+                       ;; FIXME: assuming that we have the length
+                       (end (string-to-number (match-string 3))))
+                   (cond ((< line start) nil) ; nothing found
+                         ((<= line end)       ; this is our stop
+                          (forward-line (- line start -1))
+                          (forward-char (+ column 2)))
+                         (t))))         ; keep looking
+                (t (error "Unified or context diffs only")))))))
+
 (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
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 25b41e3..91e52f8 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -710,6 +710,7 @@
 (require 'cl-lib)
 
 (declare-function diff-setup-whitespace "diff-mode" ())
+(declare-function diff-goto-line "diff-mode")
 
 (eval-when-compile
   (require 'dired))
@@ -1650,7 +1651,7 @@ to override the value of `vc-diff-switches' and `diff-switches'."
   (declare (obsolete vc-switches "22.1"))
   `(vc-switches ',backend 'diff))
 
-(defun vc-diff-finish (buffer messages)
+(defun vc-diff-finish (buffer messages loc)
   ;; The empty sync output case has already been handled, so the only
   ;; possibility of an empty output is for an async process.
   (when (buffer-live-p buffer)
@@ -1664,7 +1665,8 @@ to override the value of `vc-diff-switches' and `diff-switches'."
 	(diff-setup-whitespace)
 	(goto-char (point-min))
 	(when window
-	  (shrink-window-if-larger-than-buffer window)))
+	  (shrink-window-if-larger-than-buffer window))
+        (when loc (apply #'diff-goto-line loc)))
       (when (and messages (not emptyp))
 	(message "%sdone" (car messages))))))
 
@@ -1688,7 +1690,8 @@ Return t if the buffer had changes, nil otherwise."
 	 ;; but the only way to set it for each file included would
 	 ;; be to call the back end separately for each file.
 	 (coding-system-for-read
-	  (if files (vc-coding-system-for-diff (car files)) 'undecided)))
+	  (if files (vc-coding-system-for-diff (car files)) 'undecided))
+	 loc)
     ;; On MS-Windows and MS-DOS, Diff is likely to produce DOS-style
     ;; EOLs, which will look ugly if (car files) happens to have Unix
     ;; EOLs.
@@ -1725,6 +1728,19 @@ Return t if the buffer had changes, nil otherwise."
                      (if async 'async 1) "diff" file
                      (append (vc-switches nil 'diff) '("/dev/null"))))))
         (setq files (nreverse filtered))))
+    (unless rev2    ; remember the position in the or a current buffer
+      (let ((f files))
+        (while f
+          (let ((buf (find-buffer-visiting (car f))))
+            (when buf
+              (setq loc
+                    (with-current-buffer buf
+                      (save-restriction
+                        (widen)
+                        (list (file-relative-name (car f))
+                              (line-number-at-pos)
+                              (- (point) (line-beginning-position)))))))
+            (setq f (unless (eq buf (current-buffer)) (cdr f)))))))
     (vc-call-backend (car vc-fileset) 'diff files rev1 rev2 buffer async)
     (set-buffer buffer)
     (diff-mode)
@@ -1748,7 +1764,7 @@ Return t if the buffer had changes, nil otherwise."
       ;; after `pop-to-buffer'; the former assumes the diff buffer is
       ;; shown in some window.
       (let ((buf (current-buffer)))
-        (vc-run-delayed (vc-diff-finish buf (when verbose messages))))
+        (vc-run-delayed (vc-diff-finish buf (when verbose messages) loc)))
       ;; In the async case, we return t even if there are no differences
       ;; because we don't know that yet.
       t)))
-- 
1.6.4.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-07-19 14:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-19  5:43 [PATCH] Showing the relevant part of a diff Herring, Davis
2016-02-19  5:49 ` Lars Ingebrigtsen
2016-02-19  8:47 ` Eli Zaretskii
2016-02-20 20:20   ` Herring, Davis
2016-02-20 20:39     ` Eli Zaretskii
2016-02-20 22:49       ` Herring, Davis
2016-07-15 21:19         ` Davis Herring
2016-07-16  2:14           ` Herring, Davis
2016-07-16  9:28             ` Dmitry Gutov
2016-07-18 14:15               ` Ted Zlatanov
2016-07-18 23:20                 ` Dmitry Gutov
2016-07-19 14:12                   ` Ted Zlatanov

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.