all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#51215: Add a navigation feature to vc and diff mode: diff-goto-line
@ 2021-10-14 20:42 Uwe Brauer
  2021-10-18  1:56 ` Dmitry Gutov
  0 siblings, 1 reply; 15+ messages in thread
From: Uwe Brauer @ 2021-10-14 20:42 UTC (permalink / raw)
  To: 51215; +Cc: Davis Herring

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

Tags: patch

<#secure method=smime mode=sign>
Hi all


Attached you find a patch, in a git compatible format, (to be installed
with git apply --patch-format=hg) provided by Davis Herring with Davis's
approval of course. Davis has already signed the FSF papers.

I would like to also discuss the whole idea connected to that patch.
Please tell me whether that is appropriate, and if it is not, we might
continue the discussion on the developer list. I will send just in case
a similar message to the developer mailing list. 

A bit of history to put that into context: 5 years ago Davis Herring
send a patch (with respect to vc.el and diff-mode.el) to the devel
list, see the link for further details.

[[https://lists.gnu.org/archive/html/emacs-devel/2016-02/msg01066.html][First short vc-diff patch]]

The idea of that patch can be described best by referring to is the
diff-hg package. The function `diff-hl-diff-goto-hunk' shows the
un-committed changes in a file, and allows to jump to these changes.
Strangely enough vc-diff does not provide a similar  feature. Davis' Harris
patch, however, does introduce it.

I find it especially helpful in collaboration where push and pulling
to/from a repository, using either git or mercurial. I presume most of
the users in this mailing list are acquainted with such a workflow, but
if necessary I could provide more details.

That patch was considered as interesting but certain enhancements were
required. So Davis send a much larger patch 6 month later. That patch
however was rejected as being to large. 
[[https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00727.html][Second longer patch, cannot be merged]]

I became very interested in that feature and was not able to install
Davis's second patch (most likely to many changes in the last 5 years)

His first patch, however, I could still apply and I tested it (using
mercurial) almost for a year and it worked flawlessly for me. 

That is why I strongly recommend to apply the patch, then 

    1. Either errors are found and these will be corrected or the patch removed
    2. Or even better the feature will enhanced

But I think to reject the patch, again, would not be a bit of a shame.

regards

Uwe Brauer 




In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.14.6, Xaw3d scroll bars)
 of 2021-07-31 built on Utnapischtim
Repository revision: 83a915d3dfafd5f3d737afe1e13b75e4dd3aef96
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description: Ubuntu 16.04.7 LTS

Configured using:
 'configure --prefix=/opt/emacs28 --with-x-toolkit=athena --without-pop
 --with-mailutils'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: diff-goto-line.patch --]
[-- Type: text/patch, Size: 10872 bytes --]

# HG changeset patch
# User Davis Herring <herring@lanl.gov>
# Date 1634243435 -7200
#      Thu Oct 14 22:30:35 2021 +0200
# Node ID 18f4bbf01c0247e973b636163991fe1590459f1b
# Parent  0745c5efa8df0977a457656484c15ca4d53392c6
Add a navigation feature to vc and diff mode: diff-goto-line

* lisp/vc/vc.el (diff-goto-line): new decalare function to include the
  new diff-goto-line-functionality.

* lisp/vc/vc.el (vc-diff-finish): Modify the funcion, to suppor the
  new diff-goto-line-functionality.

* lisp/vc/diff-mode.el (diff-goto-line): New function that allows to
  jump from a specific line in the source buffer to the corresponding
  line in the buffer.

* lisp/vc/diff-mode.el (diff-hunk-header-re): modify the const, so
  that it can be used with the new diff-goto-line-functionality.

* lisp/vc/diff-mode.el (diff-hunk-header-re-context): modify the
  const, so that it can be used with the new
  diff-goto-line-functionality.

* lisp/vc/diff-mode.el (diff-hunk-header-re): Modify the const, to be
  used with new diff-goto-line-functionality.

* lisp/vc/diff-mode.el: (diff-hunk-header-re-context): New variable
  that allows to jump from a specific line in the source buffer to the
  corresponding line in the diff buffer.

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -442,8 +442,10 @@
 
 (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)
 				   (not (face-equal 'diff-changed 'diff-added))
@@ -526,7 +528,9 @@
 See https://lists.gnu.org/r/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)))
 
 (defconst diff-separator-re "^--+ ?$")
@@ -716,6 +720,46 @@
 (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 and COLUMN in FILE.
+If LINE (in the new version of FILE) is included (in the diff
+buffer as +/! line or a context line), 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
@@ -1186,7 +1230,11 @@
           (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)
@@ -1196,15 +1244,15 @@
                   (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)
@@ -1282,7 +1330,11 @@
 	(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
@@ -1646,13 +1698,13 @@
 
        ;; 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")
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -739,6 +739,7 @@
 (require 'cl-lib)
 
 (declare-function diff-setup-whitespace "diff-mode" ())
+(declare-function diff-goto-line "diff-mode")
 
 (eval-when-compile
   (require 'dired))
@@ -1716,7 +1717,7 @@
       ;; any switches in diff-switches.
       (when (listp switches) switches))))
 
-(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)
@@ -1730,7 +1731,8 @@
 	(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))))))
 
@@ -1754,7 +1756,8 @@
 	 ;; 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.
@@ -1791,6 +1794,19 @@
                      (if async 'async 1) "diff" file
                      (append (vc-switches nil 'diff) `(,(null-device)))))))
         (setq files (nreverse filtered))))
+    (unless rev2    ; remember the position in the 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)
@@ -1815,7 +1831,7 @@
       ;; 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)))

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-09-10 23:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-14 20:42 bug#51215: Add a navigation feature to vc and diff mode: diff-goto-line Uwe Brauer
2021-10-18  1:56 ` Dmitry Gutov
2021-10-18  7:16   ` Uwe Brauer
2021-10-18  7:24     ` Uwe Brauer
2021-10-19  0:49       ` Dmitry Gutov
2021-10-19  6:53         ` Juri Linkov
2021-10-19 23:18           ` Dmitry Gutov
2021-11-01  7:24           ` Uwe Brauer
2021-11-01 19:32             ` Juri Linkov
2021-11-01  8:41           ` Uwe Brauer
2022-09-10  5:13             ` Lars Ingebrigtsen
2022-09-10  5:14             ` Lars Ingebrigtsen
2022-09-10  7:15               ` Uwe Brauer
2022-09-10 23:21                 ` Dmitry Gutov
2021-10-19  0:54     ` Dmitry Gutov

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.