git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Lawrence Mitchell <wence@gmx.li>
To: "Rüdiger Sonderfeld" <ruediger@c-plusplus.de>
Cc: jrnieder@gmail.com, git@vger.kernel.org, davidk@lysator.liu.se,
	user42@zip.com.au, osv@javad.com, gitster@pobox.com
Subject: [PATCH v2 2/3] git-blame.el: Use with-current-buffer where appropriate
Date: Thu, 14 Jun 2012 10:37:59 +0100	[thread overview]
Message-ID: <1339666680-4381-2-git-send-email-wence@gmx.li> (raw)
In-Reply-To: <1339666680-4381-1-git-send-email-wence@gmx.li>

In git-blame-filter and git-blame-create-overlay we want to save
(along with the values of point and mark) the current-buffer in scope
when calling the functions.  The idiom

    (save-excursion
      (set-buffer buf)
      ...)

will correctly restore the correct buffer, but will not save the
values of point and mark in buf (only in the buffer current when the
save-excursion call is executed).  The intention of these functions is
to save the current buffer from the calling scope and the values of
point and mark in the buffer they are modifying.  The correct idiom
for this is

    (with-current-buffer buf
      (save-excursion
        ...))

Signed-off-by: Rüdiger Sonderfeld <ruediger@c-plusplus.de>
Signed-off-by: Lawrence Mitchell <wence@gmx.li>
---
 contrib/emacs/git-blame.el | 74 +++++++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

Updated commit message that actually correctly matches what Emacs
does, plus don't just squash the byte-compiler warnings but actually
fix the bug that it was pointing out to us.

For reference, here's the whitespace-squashed change:

| @@ -337,8 +337,8 @@ See also function `git-blame-mode'."
|  (defvar in-blame-filter nil)
 
|  (defun git-blame-filter (proc str)
| +  (with-current-buffer (process-buffer proc)
|      (save-excursion
| -    (set-buffer (process-buffer proc))
|        (goto-char (process-mark proc))
|        (insert-before-markers str)
|        (goto-char 0)
| @@ -346,7 +346,7 @@ See also function `git-blame-mode'."
|          (let ((more t)
|                (in-blame-filter t))
|            (while more
| -          (setq more (git-blame-parse)))))))
| +            (setq more (git-blame-parse))))))))
 
|  (defun git-blame-parse ()
|    (cond ((looking-at "\\([0-9a-f]\\{40\\}\\) \\([0-9]+\\) \\([0-9]+\\) \\([0-9]+\\)\n")
| @@ -385,8 +385,8 @@ See also function `git-blame-mode'."
|            info))))
 
|  (defun git-blame-create-overlay (info start-line num-lines)
| +  (with-current-buffer git-blame-file
|      (save-excursion
| -    (set-buffer git-blame-file)
|        (let ((inhibit-point-motion-hooks t)
|              (inhibit-modification-hooks t))
|          (goto-char (point-min))
| @@ -411,7 +411,7 @@ See also function `git-blame-mode'."
|                                             (cdr (assq 'color (cdr info))))))
|            (overlay-put ovl 'line-prefix
|                         (propertize (format-spec git-blame-prefix-format spec)
| -                                 'face 'git-blame-prefix-face))))))
| +                                   'face 'git-blame-prefix-face)))))))
 


diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
index 5428ff7..bb6d7bb 100644
--- a/contrib/emacs/git-blame.el
+++ b/contrib/emacs/git-blame.el
@@ -337,16 +337,16 @@ See also function `git-blame-mode'."
 (defvar in-blame-filter nil)
 
 (defun git-blame-filter (proc str)
-  (save-excursion
-    (set-buffer (process-buffer proc))
-    (goto-char (process-mark proc))
-    (insert-before-markers str)
-    (goto-char 0)
-    (unless in-blame-filter
-      (let ((more t)
-            (in-blame-filter t))
-        (while more
-          (setq more (git-blame-parse)))))))
+  (with-current-buffer (process-buffer proc)
+    (save-excursion
+      (goto-char (process-mark proc))
+      (insert-before-markers str)
+      (goto-char 0)
+      (unless in-blame-filter
+        (let ((more t)
+              (in-blame-filter t))
+          (while more
+            (setq more (git-blame-parse))))))))
 
 (defun git-blame-parse ()
   (cond ((looking-at "\\([0-9a-f]\\{40\\}\\) \\([0-9]+\\) \\([0-9]+\\) \\([0-9]+\\)\n")
@@ -385,33 +385,33 @@ See also function `git-blame-mode'."
           info))))
 
 (defun git-blame-create-overlay (info start-line num-lines)
-  (save-excursion
-    (set-buffer git-blame-file)
-    (let ((inhibit-point-motion-hooks t)
-          (inhibit-modification-hooks t))
-      (goto-char (point-min))
-      (forward-line (1- start-line))
-      (let* ((start (point))
-             (end (progn (forward-line num-lines) (point)))
-             (ovl (make-overlay start end))
-             (hash (car info))
-             (spec `((?h . ,(substring hash 0 6))
-                     (?H . ,hash)
-                     (?a . ,(git-blame-get-info info 'author))
-                     (?A . ,(git-blame-get-info info 'author-mail))
-                     (?c . ,(git-blame-get-info info 'committer))
-                     (?C . ,(git-blame-get-info info 'committer-mail))
-                     (?s . ,(git-blame-get-info info 'summary)))))
-        (push ovl git-blame-overlays)
-        (overlay-put ovl 'git-blame info)
-        (overlay-put ovl 'help-echo
-                     (format-spec git-blame-mouseover-format spec))
-        (if git-blame-use-colors
-            (overlay-put ovl 'face (list :background
-                                         (cdr (assq 'color (cdr info))))))
-        (overlay-put ovl 'line-prefix
-                     (propertize (format-spec git-blame-prefix-format spec)
-                                 'face 'git-blame-prefix-face))))))
+  (with-current-buffer git-blame-file
+    (save-excursion
+      (let ((inhibit-point-motion-hooks t)
+            (inhibit-modification-hooks t))
+        (goto-char (point-min))
+        (forward-line (1- start-line))
+        (let* ((start (point))
+               (end (progn (forward-line num-lines) (point)))
+               (ovl (make-overlay start end))
+               (hash (car info))
+               (spec `((?h . ,(substring hash 0 6))
+                       (?H . ,hash)
+                       (?a . ,(git-blame-get-info info 'author))
+                       (?A . ,(git-blame-get-info info 'author-mail))
+                       (?c . ,(git-blame-get-info info 'committer))
+                       (?C . ,(git-blame-get-info info 'committer-mail))
+                       (?s . ,(git-blame-get-info info 'summary)))))
+          (push ovl git-blame-overlays)
+          (overlay-put ovl 'git-blame info)
+          (overlay-put ovl 'help-echo
+                       (format-spec git-blame-mouseover-format spec))
+          (if git-blame-use-colors
+              (overlay-put ovl 'face (list :background
+                                           (cdr (assq 'color (cdr info))))))
+          (overlay-put ovl 'line-prefix
+                       (propertize (format-spec git-blame-prefix-format spec)
+                                   'face 'git-blame-prefix-face)))))))
 
 (defun git-blame-add-info (info key value)
   (nconc info (list (cons (intern key) value))))
-- 
1.7.11.rc2.9.g10afb6c

  reply	other threads:[~2012-06-14  9:38 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-12 15:44 [PATCH] git-blame.el: Fix compilation warnings Rüdiger Sonderfeld
2012-01-12 16:26 ` Jonathan Nieder
2012-01-12 17:08   ` Rüdiger Sonderfeld
2012-01-13 23:31     ` Sending patches with KMail (Re: [PATCH] git-blame.el: Fix compilation warnings.) Jonathan Nieder
2012-01-14  0:59       ` Junio C Hamano
2012-01-14 18:31         ` Sending patches with KMail Jonathan Nieder
2012-01-14 18:34           ` Jonathan Nieder
2012-01-15  2:14           ` Junio C Hamano
2012-01-14 19:18         ` Sending patches with KMail (Re: [PATCH] git-blame.el: Fix compilation warnings.) Rüdiger Sonderfeld
2012-06-10  7:38 ` [PATCH] git-blame.el: use mapc instead of mapcar Jonathan Nieder
2012-06-10 11:58   ` [PATCH 1/3] git-blame.el: Do not use goto-line in lisp code Lawrence Mitchell
2012-06-10 11:58     ` [PATCH 2/3] git-blame.el: Use with-current-buffer where appropriate Lawrence Mitchell
2012-06-10 11:58       ` [PATCH 3/3] git-blame.el: Do not use bare 0 to mean (point-min) Lawrence Mitchell
2012-06-14  5:08     ` [PATCH 1/3] git-blame.el: Do not use goto-line in lisp code Jonathan Nieder
2012-06-14  9:14       ` Lawrence Mitchell
2012-06-14  9:37         ` [PATCH v2 " Lawrence Mitchell
2012-06-14  9:37           ` Lawrence Mitchell [this message]
2012-06-14  9:38             ` [PATCH v2 3/3] git-blame.el: Do not use bare 0 to mean (point-min) Lawrence Mitchell

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1339666680-4381-2-git-send-email-wence@gmx.li \
    --to=wence@gmx.li \
    --cc=davidk@lysator.liu.se \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=osv@javad.com \
    --cc=ruediger@c-plusplus.de \
    --cc=user42@zip.com.au \
    /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 public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).