From: Davis Herring <herring@lanl.gov>
To: Eli Zaretskii <eliz@gnu.org>
Cc: Emacs development discussions <emacs-devel@gnu.org>
Subject: Re: [PATCH] Showing the relevant part of a diff
Date: Fri, 15 Jul 2016 15:19:17 -0600 [thread overview]
Message-ID: <57895355.4010803@lanl.gov> (raw)
In-Reply-To: <B393F5AD12955C48A84FFB08032CD04F6C433787@ECS-EXG-P-MB01.win.lanl.gov>
[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]
> The "no FILE" case doesn't arise for single files, of course, unless there are no changes and the whole question is moot. For changeset diffs, I (soon will) handle an absent FILE by trying another buffer's file. It is already the case that the FILE-not-found case is a no-op in that point _remains_ at the beginning; this will make it truly ignorable, so there shouldn't be any confusion.
"soon" has finally arrived. There are now several "preparatory"
patches. The first is just an update of the cleanup from before
(February), but then there's (2) another to systematize support for
normal diffs, (3) a single-word cosmetic change for multi-file diffs,
(4) a refactor of `vc-parent-buffer' code, and (5) a documentation fix
and simplification for `get-buffer-window' and friends.
The main (last) patch addresses the questions I asked and issues raised
by Lars and Eli: it provides NEWS and manual text, supports
normal-format diffs (as Lars said "would be nice"), and supports
`vc-root-diff', `diff-backup', and `diff-buffer-with-file'. (I could
easily support `diff' itself too; it does default to comparing to the
current buffer's file, after all.) A buffer's point is used only if the
buffer is visible, so as to not surprise the user by drawing attention
to a part of a file that is no longer of concern.
It now has one code path for all diff formats, but it still assumes that
hunk headers are accurate (is that a problem?), and it still has the
behavior (I call it a feature) of showing you the place where a hunk
describing a change at your current position would go if there were such
a change.
Thoughts?
Davis
--
This product is sold by volume, not by mass. If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.
[-- Attachment #2: 0001-Clean-up-context-diff-header-regexps.patch --]
[-- Type: text/x-patch, Size: 5271 bytes --]
From 4fd6ec538e4e1b73645569283ed1431bbf3a3a98 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/6] Clean up context diff header regexps
* lisp/vc/diff-mode.el (diff-hunk-header-re-context): New const.
(diff-hunk-header-re,diff-context->unified,diff-reverse-direction,
diff-sanity-check-hunk): Use it.
(diff-context-mid-hunk-header-re): Add missing ^.
---
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 58498cb..10a2734 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -359,8 +359,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)
(not (face-equal 'diff-changed 'diff-added))
@@ -435,7 +437,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)
@@ -1028,7 +1032,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)
@@ -1038,15 +1046,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)
@@ -1124,7 +1132,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
@@ -1477,13 +1489,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.7.1
[-- Attachment #3: 0002-Add-constants-for-normal-diff-headers.patch --]
[-- Type: text/x-patch, Size: 4394 bytes --]
From b2da41327b717fbceed925a8062081af38165639 Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Sun, 19 Jun 2016 02:00:50 -0600
Subject: [PATCH 2/6] Add constants for normal diff headers
* lisp/vc/diff-mode.el (diff-hunk-header-re-normal,
diff-normal-mid-hunk-header-re): New consts.
(diff-font-lock-keywords, diff-hunk-header-re, diff-hunk-text,
diff-refine-hunk): Use them.
---
lisp/vc/diff-mode.el | 41 ++++++++++++++++++++---------------------
1 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 10a2734..b859c5e 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -361,8 +361,12 @@ well."
"^@@ -\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? \\+\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? @@")
(defconst diff-hunk-header-re-context
"\\*\\{15\\}\\(?: .*\\)?\n\\*\\*\\* \\(\\([0-9]+\\)\\(?:,\\(-?[0-9]+\\)\\)?\\) \\*\\*\\*\\*")
+(defconst diff-hunk-header-re-normal
+ (let ((pair "\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)?"))
+ (format "^%s\\([acd]\\)%s$" pair pair)))
(defconst diff-context-mid-hunk-header-re
"^--- \\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? ----$")
+(defconst diff-normal-mid-hunk-header-re "^---$")
(defvar diff-use-changed-face (and (face-differs-from-default-p 'diff-changed)
(not (face-equal 'diff-changed 'diff-added))
@@ -378,8 +382,8 @@ and the face `diff-added' for added lines.")
(1 'diff-hunk-header) (2 'diff-function))
("^\\*\\*\\* .+ \\*\\*\\*\\*". 'diff-hunk-header) ;context
(,diff-context-mid-hunk-header-re . 'diff-hunk-header) ;context
- ("^[0-9,]+[acd][0-9,]+$" . 'diff-hunk-header) ;normal
- ("^---$" . 'diff-hunk-header) ;normal
+ (,diff-hunk-header-re-normal . 'diff-hunk-header)
+ (,diff-normal-mid-hunk-header-re . 'diff-hunk-header)
;; For file headers, accept files with spaces, but be careful to rule
;; out false-positives when matching hunk headers.
("^\\(---\\|\\+\\+\\+\\|\\*\\*\\*\\) \\([^\t\n]+?\\)\\(?:\t.*\\| \\(\\*\\*\\*\\*\\|----\\)\\)?\n"
@@ -439,7 +443,7 @@ See http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01990.html")
(defconst diff-hunk-header-re
(concat "^\\(?:" diff-hunk-header-re-unified ".*\\|"
diff-hunk-header-re-context
- "\\|[0-9]+\\(,[0-9]+\\)?[acd][0-9]+\\(,[0-9]+\\)?\\)$"))
+ "\\|" diff-hunk-header-re-normal "\\)$"))
(defconst diff-file-header-re (concat "^\\(--- .+\n\\+\\+\\+ \\|\\*\\*\\* .+\n--- \\|[^-+!<>0-9@* \n]\\).+\n" (substring diff-hunk-header-re 1)))
(defvar diff-narrowed-to nil)
@@ -1582,23 +1586,18 @@ char-offset in TEXT."
(setq divider-pos (point))
(forward-line 1)
(setq dst-pos (point)))
- ((looking-at "^[0-9]+a[0-9,]+$")
- ;; normal diff, insert
- (forward-line 1)
- (setq dst-pos (point)))
- ((looking-at "^[0-9,]+d[0-9]+$")
- ;; normal diff, delete
- (forward-line 1)
- (setq src-pos (point)))
- ((looking-at "^[0-9,]+c[0-9,]+$")
- ;; normal diff, change
- (forward-line 1)
- (setq src-pos (point))
- (re-search-forward "^---$" nil t)
- (forward-line 0)
- (setq divider-pos (point))
- (forward-line 1)
- (setq dst-pos (point)))
+ ((looking-at diff-hunk-header-re-normal)
+ (forward-line 1)
+ (pcase (match-string 3)
+ ("a" (setq dst-pos (point))) ; insert
+ ("d" (setq src-pos (point))) ; delete
+ (_ ; otherwise change
+ (setq src-pos (point))
+ (re-search-forward diff-normal-mid-hunk-header-re nil t)
+ (forward-line 0)
+ (setq divider-pos (point))
+ (forward-line 1)
+ (setq dst-pos (point)))))
(t
(error "Unknown diff hunk type")))
@@ -2014,7 +2013,7 @@ For use in `add-log-current-defun-function'."
(unless diff-use-changed-face props-a)))))
(_ ;; Normal diffs.
(let ((beg1 (1+ (point))))
- (when (re-search-forward "^---.*\n" end t)
+ (when (re-search-forward diff-normal-mid-hunk-header-re end t)
;; It's a combined add&remove, so there's something to do.
(smerge-refine-subst beg1 (match-beginning 0)
(match-end 0) end
--
1.7.1
[-- Attachment #4: 0003-Use-checkout-rather-than-workfile.patch --]
[-- Type: text/x-patch, Size: 1078 bytes --]
From f9e67a7e71aa421cb9d0f282ffaae476692b9541 Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Sat, 11 Jun 2016 15:21:01 -0600
Subject: [PATCH 3/6] Use "checkout" rather than "workfile"
A singular "workfile" is wrong for comparing a directory.
* lisp/vc/vc.el (vc-diff-internal): Change message.
---
lisp/vc/vc.el | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index af875e8..343cd70 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1683,7 +1683,7 @@ Return t if the buffer had changes, nil otherwise."
(vc-delistify files))
(format "No changes between %s and %s"
(or rev1 "working revision")
- (or rev2 "workfile"))))
+ (or rev2 "checkout"))))
;; Set coding system based on the first file. It's a kluge,
;; but the only way to set it for each file included would
;; be to call the back end separately for each file.
--
1.7.1
[-- Attachment #5: 0004-Snap-vc-parent-buffer-chains.patch --]
[-- Type: text/x-patch, Size: 3886 bytes --]
From 0e0c14b0d8ff8582301eab65c91120a52aad5597 Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Sun, 12 Jun 2016 05:37:49 -0600
Subject: [PATCH 4/6] Snap `vc-parent-buffer' chains
This avoids having to search the tree and check for loops.
* lisp/vc/vc-dispatcher.el (vc-get-parent-buffer, vc-set-parent-buffer):
New functions.
(vc-setup-buffer, vc-start-logentry): Use `vc-set-parent-buffer'.
* lisp/vc/vc.el (vc-ensure-vc-buffer, vc-find-revision): Use both.
---
lisp/vc/vc-dispatcher.el | 22 ++++++++++++++++------
lisp/vc/vc.el | 9 ++-------
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index a551542..f39d517 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -166,6 +166,20 @@ Another is that undo information is not kept."
(insert s)
(set-marker (process-mark p) (point))))))))
+(defun vc-get-parent-buffer ()
+ "Return the current buffer or its parent if it has one."
+ (if (buffer-live-p vc-parent-buffer) vc-parent-buffer (current-buffer)))
+(defun vc-set-parent-buffer (buf &optional silent)
+ "Set current buffer's parent to BUF (or its parent if it has one).
+Do nothing if that is the current buffer.
+Set the mode line name as well unless SILENT."
+ (setq buf (with-current-buffer buf (vc-get-parent-buffer)))
+ (unless (eq buf (current-buffer))
+ (set (make-local-variable 'vc-parent-buffer) buf)
+ (unless silent
+ (set (make-local-variable 'vc-parent-buffer-name)
+ (concat " from " (buffer-name buf))))))
+
(defun vc-setup-buffer (buf)
"Prepare BUF for executing a slave command and make it current."
(let ((camefrom (current-buffer))
@@ -178,9 +192,7 @@ Another is that undo information is not kept."
;; want any of its output to appear from now on.
(when oldproc (delete-process oldproc)))
(kill-all-local-variables)
- (set (make-local-variable 'vc-parent-buffer) camefrom)
- (set (make-local-variable 'vc-parent-buffer-name)
- (concat " from " (buffer-name camefrom)))
+ (vc-set-parent-buffer camefrom)
(setq default-directory olddir)
(let ((buffer-undo-list t)
(inhibit-read-only t))
@@ -662,9 +674,7 @@ BACKEND, if non-nil, specifies a VC backend for the Log Edit buffer."
(if (and comment (not initial-contents))
(set-buffer (get-buffer-create logbuf))
(pop-to-buffer (get-buffer-create logbuf)))
- (set (make-local-variable 'vc-parent-buffer) parent)
- (set (make-local-variable 'vc-parent-buffer-name)
- (concat " from " (buffer-name vc-parent-buffer)))
+ (vc-set-parent-buffer parent)
(vc-log-edit files mode backend)
(make-local-variable 'vc-log-after-operation-hook)
(when after-hook
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 343cd70..bb5cdd5 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1069,12 +1069,7 @@ BEWARE: this function may change the current buffer."
((derived-mode-p 'vc-dir-mode)
(set-buffer (find-file-noselect (vc-dir-current-file))))
(t
- (while (and vc-parent-buffer
- (buffer-live-p vc-parent-buffer)
- ;; Avoid infinite looping when vc-parent-buffer and
- ;; current buffer are the same buffer.
- (not (eq vc-parent-buffer (current-buffer))))
- (set-buffer vc-parent-buffer))
+ (set-buffer (vc-get-parent-buffer))
(if (not buffer-file-name)
(error "Buffer %s is not associated with a file" (buffer-name))
(unless (vc-backend buffer-file-name)
@@ -1978,7 +1973,7 @@ Use BACKEND as the VC backend if specified."
(with-current-buffer result-buf
;; Set the parent buffer so that things like
;; C-x v g, C-x v l, ... etc work.
- (set (make-local-variable 'vc-parent-buffer) filebuf))
+ (vc-set-parent-buffer filebuf t))
result-buf)))
;; Header-insertion code
--
1.7.1
[-- Attachment #6: 0005-Document-all-frames-argument-once.patch --]
[-- Type: text/x-patch, Size: 5209 bytes --]
From e169fd4102cc82bfb19726759af1913496024fb6 Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Wed, 6 Jul 2016 07:30:49 -0600
Subject: [PATCH 5/6] Document `all-frames' argument once
* src/window.c (candidate_window_p, Fnext_window, Fprevious_window,
Fget_buffer_window): Rephrase documentation and comments in terms of
Fwindow_list_1.
(window_list_1): Remove reference to `next-window'.
(window_loop): Phrase comment in terms of Fget_buffer_window.
---
src/window.c | 72 ++++++++-------------------------------------------------
1 files changed, 10 insertions(+), 62 deletions(-)
diff --git a/src/window.c b/src/window.c
index e123b89..2701ed9 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2318,14 +2318,10 @@ struct Lisp_Char_Table *
`lambda' means WINDOW may not be a minibuffer window.
a window means a specific minibuffer window
- ALL_FRAMES t means search all frames,
- nil means search just current frame,
- `visible' means search just visible frames on the
- current terminal,
- 0 means search visible and iconified frames on the
- current terminal,
- a window means search the frame that window belongs to,
- a frame means consider windows on that frame, only. */
+ ALL_FRAMES as in `window-list-1', except that nil is equivalent
+ to OWINDOW's frame; additionally, a window means
+ search its frame and any others that use it as a
+ minibuffer or have its frame as a focus frame. */
static bool
candidate_window_p (Lisp_Object window, Lisp_Object owindow,
@@ -2506,23 +2502,7 @@ struct Lisp_Char_Table *
even if the minibuffer is not active. Any other value means do not
consider the minibuffer window even if the minibuffer is active.
-ALL-FRAMES nil or omitted means consider all windows on WINDOW's frame,
-plus the minibuffer window if specified by the MINIBUF argument. If the
-minibuffer counts, consider all windows on all frames that share that
-minibuffer too. The following non-nil values of ALL-FRAMES have special
-meanings:
-
-- t means consider all windows on all existing frames.
-
-- `visible' means consider all windows on all visible frames.
-
-- 0 (the number zero) means consider all windows on all visible and
- iconified frames.
-
-- A frame means consider all windows on that frame only.
-
-Anything else means consider all windows on WINDOW's frame and no
-others.
+See `window-list-1' for the meaning of ALL-FRAMES.
If you use consistent values for MINIBUF and ALL-FRAMES, you can use
`next-window' to iterate through the entire cycle of acceptable
@@ -2545,23 +2525,7 @@ struct Lisp_Char_Table *
even if the minibuffer is not active. Any other value means do not
consider the minibuffer window even if the minibuffer is active.
-ALL-FRAMES nil or omitted means consider all windows on WINDOW's frame,
-plus the minibuffer window if specified by the MINIBUF argument. If the
-minibuffer counts, consider all windows on all frames that share that
-minibuffer too. The following non-nil values of ALL-FRAMES have special
-meanings:
-
-- t means consider all windows on all existing frames.
-
-- `visible' means consider all windows on all visible frames.
-
-- 0 (the number zero) means consider all windows on all visible and
- iconified frames.
-
-- A frame means consider all windows on that frame only.
-
-Anything else means consider all windows on WINDOW's frame and no
-others.
+See `window-list-1' for the meaning of ALL-FRAMES.
If you use consistent values for MINIBUF and ALL-FRAMES, you can
use `previous-window' to iterate through the entire cycle of
@@ -2574,9 +2538,6 @@ struct Lisp_Char_Table *
}
-/* Return a list of windows in cyclic ordering. Arguments are like
- for `next-window'. */
-
static Lisp_Object
window_list_1 (Lisp_Object window, Lisp_Object minibuf, Lisp_Object all_frames)
{
@@ -2664,10 +2625,7 @@ struct Lisp_Char_Table *
\f
/* Look at all windows, performing an operation specified by TYPE
with argument OBJ.
- If FRAMES is Qt, look at all frames;
- Qnil, look at just the selected frame;
- Qvisible, look at visible frames;
- a frame, just look at windows on that frame.
+ FRAMES is like ALL_FRAMES for `get-buffer-window'.
If MINI, perform the operation on minibuffer windows too. */
enum window_loop
@@ -2829,19 +2787,9 @@ enum window_loop
BUFFER-OR-NAME may be a buffer or a buffer name and defaults to
the current buffer.
-The optional argument ALL-FRAMES specifies the frames to consider:
-
-- t means consider all windows on all existing frames.
-
-- `visible' means consider all windows on all visible frames.
-
-- 0 (the number zero) means consider all windows on all visible
- and iconified frames.
-
-- A frame means consider all windows on that frame only.
-
-Any other value of ALL-FRAMES means consider all windows on the
-selected frame and no others. */)
+See `window-list-1' for the meaning of the optional argument
+ALL-FRAMES, except that nil is not a special value; the selected
+window is used for WINDOW. */)
(Lisp_Object buffer_or_name, Lisp_Object all_frames)
{
Lisp_Object buffer;
--
1.7.1
[-- Attachment #7: 0006-Set-point-in-diff-to-correspond-to-point-in-file.patch --]
[-- Type: text/x-patch, Size: 18462 bytes --]
From ce3df56ecc0e16b11106e2f7c6c9246345cfca15 Mon Sep 17 00:00:00 2001
From: Davis Herring <herring@lanl.gov>
Date: Tue, 12 Jul 2016 22:35:01 -0600
Subject: [PATCH 6/6] Set point in diff to correspond to point in file
* lisp/vc/diff-mode.el (diff-location, diff-goto-line): New functions.
* lisp/vc/diff.el (diff-sentinel): Add BUF and FUNC arguments.
Move point in (some) window showing the diff.
(diff, diff-no-select): Add FUNC argument to pass to `diff-sentinel'.
(diff-no-select): Call `diff-sentinel' only on exit.
Pass buffer rather than selecting it.
(diff-buffer-current): New function.
(diff-backup): Arrange to call `diff-goto-line' if a buffer is current.
(diff-buffer-with-file): Arrange to call it if buffer is visible.
* lisp/vc/vc.el (vc-diff-finish): Use it with new LOC argument.
(vc-diff-internal): Note location and pass it to `vc-diff-finish'.
* doc/emacs/files.texi, doc/emacs/maintaining.texi: Document point
placement.
---
doc/emacs/files.texi | 7 +++-
doc/emacs/maintaining.texi | 10 +++--
etc/NEWS | 7 +++-
lisp/vc/diff-mode.el | 65 ++++++++++++++++++++++++++++++++++++++
lisp/vc/diff.el | 75 +++++++++++++++++++++++++++++++-------------
lisp/vc/vc.el | 36 +++++++++++++++++++--
6 files changed, 168 insertions(+), 32 deletions(-)
diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index f195a41..ebfe2df 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1291,12 +1291,17 @@ called Diff mode. @xref{Diff Mode}.
The command @kbd{M-x diff-backup} compares a specified file with its
most recent backup. If you specify the name of a backup file,
@code{diff-backup} compares it with the source file that it is a
-backup of. In all other respects, this behaves like @kbd{M-x diff}.
+backup of. If there is a visible, up-to-date buffer visiting the
+file, point is placed in the @file{*diff*} buffer at the place
+corresponding to point in that buffer. In all other respects, this
+behaves like @kbd{M-x diff}.
@findex diff-buffer-with-file
The command @kbd{M-x diff-buffer-with-file} compares a specified
buffer with its corresponding file. This shows you what changes you
would make to the file if you save the buffer.
+Point is placed in @file{*diff*} to correspond to point in the buffer
+if it is visible.
@findex compare-windows
The command @kbd{M-x compare-windows} compares the text in the
diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 1037bd1..11bf19c 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -792,10 +792,12 @@ the latest revision in which it was modified (@code{vc-annotate}).
@kbd{C-x v =} (@code{vc-diff}) displays a @dfn{diff} which compares
each work file in the current VC fileset to the version(s) from which
you started editing. The diff is displayed in another window, in a
-Diff mode buffer (@pxref{Diff Mode}) named @file{*vc-diff*}. The
-usual Diff mode commands are available in this buffer. In particular,
-the @kbd{g} (@code{revert-buffer}) command performs the file
-comparison again, generating a new diff.
+Diff mode buffer (@pxref{Diff Mode}) named @file{*vc-diff*}, with
+point placed at the location corresponding to point in a visible,
+up-to-date buffer visiting one of the files being compared (preferring
+the current buffer). The usual Diff mode commands are available in
+this buffer. In particular, the @kbd{g} (@code{revert-buffer})
+command performs the file comparison again, generating a new diff.
@kindex C-u C-x v =
To compare two arbitrary revisions of the current VC fileset, call
diff --git a/etc/NEWS b/etc/NEWS
index c58349c..5b446cd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -319,11 +319,16 @@ provided: 'image-property'.
directory suffixes (gnu, gnu/lib, gnu/lib/emacs, emacs, lib, lib/emacs)
when searching for info directories.
+** VC and related modes
+++
-** The commands that add ChangeLog entries now prefer a VCS root directory
+*** The commands that add ChangeLog entries now prefer a VCS root directory
for the ChangeLog file, if none already exists. Customize
'change-log-directory-files' to nil for the old behavior.
++++
+*** Diffs showing the changes made in a visible file are displayed
+with point set to correspond to point in the visiting buffer.
+
---
** Support for non-string values of 'time-stamp-format' has been removed.
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index b859c5e..b715d75 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -580,6 +580,71 @@ 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-location (&optional pwd)
+ "Return location information for `diff-goto-line'.
+The result looks like (FILE LINE COLUMN).
+FILE is relative to PWD if it is non-nil."
+ (save-restriction
+ (widen)
+ (list (if pwd (file-relative-name buffer-file-name pwd)
+ buffer-file-name)
+ (line-number-at-pos)
+ (- (point) (line-beginning-position)))))
+
+(defun diff-goto-line (file line column)
+ "Go to the place in this diff producing LINE and COLUMN in FILE.
+If LINE appears in a hunk (as an added, changed, or 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 (save-match-data (diff-find-file-name nil t))
+ file))))
+ (if (eobp) (goto-char (point-min))
+ (let ((limit (save-match-data
+ (save-excursion
+ (and (re-search-forward diff-file-header-re nil t)
+ (match-beginning 0)))))
+ diff-auto-refine-mode)
+ (goto-char (match-beginning 0))
+ (while
+ (and
+ (if (progn (forward-line)
+ (re-search-forward diff-hunk-header-re limit t))
+ (goto-char (match-beginning 0))
+ (diff-end-of-hunk) nil) ; end of the line
+ ;; FIXME: assuming that the endpoint is always known from header
+ (let (start end unified (beg (point)))
+ (cond
+ ((looking-at diff-hunk-header-re-unified)
+ (setq start (string-to-number (match-string 3))
+ end (+ start (string-to-number (match-string 4)) -1)
+ unified t))
+ ((looking-at diff-hunk-header-re-context)
+ (re-search-forward diff-context-mid-hunk-header-re)
+ (setq start (string-to-number (match-string 1))
+ end (string-to-number (match-string 2))))
+ ((looking-at diff-hunk-header-re-normal)
+ (setq start (string-to-number (match-string 4))
+ end (let ((e (match-string 5)))
+ (if e (string-to-number e) start)))
+ (pcase (match-string 3)
+ ("d" (setq end (1- end)))
+ ("c" (re-search-forward diff-normal-mid-hunk-header-re))))
+ (t (error "Bad header")))
+ (cond
+ ((< line start) (goto-char beg) nil) ; nothing found
+ ((<= line end) ; this is our stop
+ (let ((dist (- line start -1)))
+ (if unified
+ (dotimes (_ dist)
+ (while (progn (forward-line) (eq (char-after) ?-))))
+ (forward-line dist)))
+ (forward-char (+ column (if unified 1 2)))
+ nil)
+ (t)))))))) ; keep looking
+
(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/diff.el b/lisp/vc/diff.el
index 6b316c4..515c8e1 100644
--- a/lisp/vc/diff.el
+++ b/lisp/vc/diff.el
@@ -32,6 +32,8 @@
;;; Code:
(declare-function diff-setup-whitespace "diff-mode" ())
+(declare-function diff-location "diff-mode" (&optional pwd))
+(declare-function diff-goto-line "diff-mode")
(defgroup diff nil
"Comparing files with `diff'."
@@ -57,32 +59,45 @@
diff-switches
(mapconcat 'identity diff-switches " ")))))
-(defun diff-sentinel (code &optional old-temp-file new-temp-file)
+(defun diff-sentinel (buf code &optional old-temp-file new-temp-file func)
"Code run when the diff process exits.
+BUF is the diff output buffer.
CODE is the exit code of the process. It should be 0 only if no diffs
were found.
If optional args OLD-TEMP-FILE and/or NEW-TEMP-FILE are non-nil,
-delete the temporary files so named."
+delete the temporary files so named.
+Call FUNC with no arguments if it is non-nil and BUF has not been killed.
+BUF and its window (if any) are current when FUNC is called."
(if old-temp-file (delete-file old-temp-file))
(if new-temp-file (delete-file new-temp-file))
- (diff-setup-whitespace)
- (goto-char (point-min))
- (save-excursion
- (goto-char (point-max))
- (let ((inhibit-read-only t))
- (insert (format "\nDiff finished%s. %s\n"
- (cond ((equal 0 code) " (no differences)")
- ((equal 2 code) " (diff error)")
- (t ""))
- (current-time-string))))))
+ (when (buffer-name buf)
+ ;; Select a (nearby) window showing BUF to make point changes permanent.
+ (save-selected-window ; also buffer
+ (let ((w (or (and (eq (window-buffer) buf) (selected-window))
+ (get-buffer-window buf)
+ (get-buffer-window buf 'visible)
+ (get-buffer-window buf t))))
+ (if w (select-window w) (set-buffer buf)))
+ (diff-setup-whitespace)
+ (goto-char (point-min))
+ (save-excursion
+ (goto-char (point-max))
+ (let ((inhibit-read-only t))
+ (insert (format "\nDiff finished%s. %s\n"
+ (cond ((equal 0 code) " (no differences)")
+ ((equal 2 code) " (diff error)")
+ (t ""))
+ (current-time-string)))))
+ (when func (funcall func)))))
;;;###autoload
-(defun diff (old new &optional switches no-async)
+(defun diff (old new &optional switches no-async func)
"Find and display the differences between OLD and NEW files.
When called interactively, read NEW, then OLD, using the
minibuffer. The default for NEW is the current buffer's file
name, and the default for OLD is a backup file for NEW, if one
exists. If NO-ASYNC is non-nil, call diff synchronously.
+See `diff-sentinel' for the meaning of FUNC.
When called interactively with a prefix argument, prompt
interactively for diff switches. Otherwise, the switches
@@ -104,7 +119,7 @@ specified in the variable `diff-switches' are passed to the diff command."
(file-name-directory newf) nil t)))
(list oldf newf (diff-switches))))
(display-buffer
- (diff-no-select old new switches no-async)))
+ (diff-no-select old new switches no-async nil func)))
(defun diff-file-local-copy (file-or-buf)
(if (bufferp file-or-buf)
@@ -121,7 +136,7 @@ Possible values are:
nil -- no, it does not
check -- try to probe whether it does")
-(defun diff-no-select (old new &optional switches no-async buf)
+(defun diff-no-select (old new &optional switches no-async buf func)
;; Noninteractive helper for creating and reverting diff buffers
(unless (bufferp new) (setq new (expand-file-name new)))
(unless (bufferp old) (setq old (expand-file-name old)))
@@ -163,7 +178,8 @@ Possible values are:
(diff-mode)
(set (make-local-variable 'revert-buffer-function)
(lambda (_ignore-auto _noconfirm)
- (diff-no-select old new switches no-async (current-buffer))))
+ (diff-no-select old new switches no-async
+ (current-buffer) func)))
(setq default-directory thisdir)
(let ((inhibit-read-only t))
(insert command "\n"))
@@ -173,15 +189,17 @@ Possible values are:
(set-process-filter proc 'diff-process-filter)
(set-process-sentinel
proc (lambda (proc _msg)
- (with-current-buffer (process-buffer proc)
- (diff-sentinel (process-exit-status proc)
- old-alt new-alt)))))
+ (unless (process-live-p proc)
+ (diff-sentinel (process-buffer proc)
+ (process-exit-status proc)
+ old-alt new-alt func)))))
;; Async processes aren't available.
(let ((inhibit-read-only t))
(diff-sentinel
+ (current-buffer)
(call-process shell-file-name nil buf nil
shell-command-switch command)
- old-alt new-alt))))
+ old-alt new-alt func))))
buf))
(defun diff-process-filter (proc string)
@@ -195,6 +213,12 @@ Possible values are:
(set-marker (process-mark proc) (point)))
(if moving (goto-char (process-mark proc))))))
+(defun diff-buffer-current (&optional buffer)
+ "Return non-nil if BUFFER is displayed and corresponds to its file."
+ (and (not (buffer-modified-p buffer))
+ (get-buffer-window buffer 'visible)
+ (verify-visited-file-modtime buffer)))
+
;;;###autoload
(defun diff-backup (file &optional switches)
"Diff this file with its backup file or vice versa.
@@ -211,7 +235,11 @@ With prefix arg, prompt for diff switches."
(setq bak (or (diff-latest-backup-file file)
(error "No backup found for %s" file))
ori file))
- (diff bak ori switches)))
+ (diff bak ori switches nil
+ (let ((b (find-buffer-visiting ori)))
+ (and b (diff-buffer-current b)
+ (let ((loc (with-current-buffer b (diff-location))))
+ (lambda () (apply 'diff-goto-line loc))))))))
;;;###autoload
(defun diff-latest-backup-file (fn)
@@ -227,7 +255,10 @@ With prefix arg, prompt for diff switches."
This requires the external program `diff' to be in your `exec-path'."
(interactive "bBuffer: ")
(with-current-buffer (get-buffer (or buffer (current-buffer)))
- (diff buffer-file-name (current-buffer) nil 'noasync)))
+ (diff buffer-file-name (current-buffer) nil 'noasync
+ (and (get-buffer-window nil 'visible)
+ (let ((loc (diff-location)))
+ (lambda () (apply 'diff-goto-line loc)))))))
(provide 'diff)
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index bb5cdd5..589cdac 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -709,7 +709,10 @@
(require 'vc-dispatcher)
(require 'cl-lib)
+(declare-function diff-buffer-current "diff" (&optional buf))
(declare-function diff-setup-whitespace "diff-mode" ())
+(declare-function diff-location "diff-mode" (pwd))
+(declare-function diff-goto-line "diff-mode")
(eval-when-compile
(require 'dired))
@@ -1645,7 +1648,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)
@@ -1659,7 +1662,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))))))
@@ -1683,7 +1687,9 @@ 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))
+ (from (current-buffer))
+ 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.
@@ -1720,6 +1726,28 @@ 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
+ (require 'diff)
+ (require 'diff-mode)
+ ;; Remember position in the most recent relevant buffer.
+ ;; Put the (original) current buffer first like `select-window' does.
+ (let (regs dirs
+ (bufs (cons from (buffer-list)))
+ (pwd default-directory))
+ (dolist (f files)
+ (push (file-truename f) (if (file-directory-p f) dirs regs)))
+ (while bufs
+ (with-current-buffer (car bufs)
+ (let ((f buffer-file-truename))
+ (and (vc-backend f)
+ (or (member f regs) ; visiting a file in the set
+ (let ((d dirs)) ; or a file in a directory in the set
+ (while (and d (not (string-prefix-p (car d) f)))
+ (setq d (cdr d)))
+ d))
+ (diff-buffer-current)
+ (setq loc (diff-location pwd)))))
+ (setq bufs (unless loc (cdr bufs))))))
(vc-call-backend (car vc-fileset) 'diff files rev1 rev2 buffer async)
(set-buffer buffer)
(diff-mode)
@@ -1743,7 +1771,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.7.1
next prev parent reply other threads:[~2016-07-15 21:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57895355.4010803@lanl.gov \
--to=herring@lanl.gov \
--cc=eliz@gnu.org \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.