GNU bug report logs - #11319
[PATCH 1/2] BUGFIX: select current-diff-comparison regions for ediff-inferior-compare-regions correctly

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11319 in the body.
You can then email your comments to 11319 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dave Abrahams <dave <at> boostpro.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Dave Abrahams <dave <at> boostpro.com>
Subject: [PATCH 1/2] BUGFIX: select current-diff-comparison regions for
	ediff-inferior-compare-regions correctly
Date: Mon, 23 Apr 2012 15:28:25 -0400

The buffer types passed to
ediff-clone-buffer-for-current-diff-comparison are used to select the
character ranges that participate in the inferior comparison.  Those
were being hardcoded as 'A and 'B, which was wrong.
---
 lisp/vc/ediff-util.el |   20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
index 78a2163..046070d 100644
--- a/lisp/vc/ediff-util.el
+++ b/lisp/vc/ediff-util.el
@@ -3378,10 +3378,18 @@ Without an argument, it saves customized diff argument, if available
     (set-window-buffer wind cloned-buff)
     cloned-buff))
 
-(defun ediff-clone-buffer-for-current-diff-comparison (buff buf-type reg-name)
-  (let ((cloned-buff (ediff-make-cloned-buffer buff reg-name))
-	(reg-start (ediff-get-diff-posn buf-type 'beg))
-	(reg-end (ediff-get-diff-posn buf-type 'end)))
+(defun ediff-buffer-type (buffer)
+  (cond ((eq buffer ediff-buffer-A) 'A)
+        ((eq buffer ediff-buffer-B) 'B)
+        ((eq buffer ediff-buffer-C) 'C)
+        ((eq buffer ediff-ancestor-buffer) 'Ancestor)
+        (t nil)))
+
+(defun ediff-clone-buffer-for-current-diff-comparison (buff reg-name)
+  (let* ((cloned-buff (ediff-make-cloned-buffer buff reg-name))
+         (buf-type (ediff-buffer-type buff))
+         (reg-start (ediff-get-diff-posn buf-type 'beg))
+         (reg-end (ediff-get-diff-posn buf-type 'end)))
     (ediff-with-current-buffer cloned-buff
       ;; set region to be the current diff region
       (goto-char reg-start)
@@ -3555,7 +3563,7 @@ Ediff Control Panel to restore highlighting."
 
     (setq bufA (if use-current-diff-p
 		   (ediff-clone-buffer-for-current-diff-comparison
-		    bufA 'A "-Region.A-")
+		    bufA "-Region.A-")
 		 (ediff-clone-buffer-for-region-comparison bufA "-Region.A-")))
     (ediff-with-current-buffer bufA
       (setq begA (region-beginning)
@@ -3570,7 +3578,7 @@ Ediff Control Panel to restore highlighting."
 
     (setq bufB (if use-current-diff-p
 		   (ediff-clone-buffer-for-current-diff-comparison
-		    bufB 'B "-Region.B-")
+		    bufB "-Region.B-")
 		 (ediff-clone-buffer-for-region-comparison bufB "-Region.B-")))
     (ediff-with-current-buffer bufB
       (setq begB (region-beginning)
-- 
1.7.10

Message #16 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dave Abrahams <dave <at> boostpro.com>
Cc: Michael Kifer <kifer <at> cs.stonybrook.edu>, 11319-done <at> debbugs.gnu.org
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Fri, 14 Dec 2012 10:23:53 -0500

Version: 24.4

> The buffer types passed to
> ediff-clone-buffer-for-current-diff-comparison are used to select the
> character ranges that participate in the inferior comparison.  Those
> were being hardcoded as 'A and 'B, which was wrong.

Thank you, it looks OK, so I installed it in trunk,


        Stefan

Message #34 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Dave Abrahams <dave <at> boostpro.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Kifer <kifer <at> cs.stonybrook.edu>, 11319-done <at> debbugs.gnu.org
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Fri, 14 Dec 2012 12:17:00 -0500

on Fri Dec 14 2012, Stefan Monnier <monnier-AT-iro.umontreal.ca> wrote:

> Version: 24.4
>
>> The buffer types passed to
>> ediff-clone-buffer-for-current-diff-comparison are used to select the
>> character ranges that participate in the inferior comparison.  Those
>> were being hardcoded as 'A and 'B, which was wrong.
>
> Thank you, it looks OK, so I installed it in trunk,
>
>         Stefan

Fantastic!  Now what about #11320, which depends on this one?  It takes
a feature from being completely useless and confusing to being
wonderful.  I'd love to be able to throw away my override of that file.

-- 
Dave Abrahams
BoostPro Computing                  Software Development        Training
http://www.boostpro.com             Clang/LLVM/EDG Compilers  C++  Boost

Message #37 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Kifer <michael.kifer <at> stonybrook.edu>
To: Dave Abrahams <dave <at> boostpro.com>
Cc: 11319-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Fri, 14 Dec 2012 23:46:17 -0500

[Message part 1 (text/html, inline)]

Message #40 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Dave Abrahams <dave <at> boostpro.com>
To: Michael Kifer <michael.kifer <at> stonybrook.edu>
Cc: 11319-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Mon, 17 Dec 2012 18:22:15 -0500

on Fri Dec 14 2012, Michael Kifer <michael.kifer-AT-stonybrook.edu> wrote:

> I think it is a good patch. One question: shouldn't the message
> "Which buffer to compare to the merge buffer (A or B)? ")
> be then changed to this?
> "Which buffer to compare to the ancestor buffer (A or B)? ")
>
> This is not part of the  #11320 patch.

I have no opinion about the change from "merge" to "ancestor."  It makes
absolutely no difference to me which of those words Emacs uses; I'll
interpret it exactly the same way.  I suggest you accept the patch as-is
and open a separate issue if you think the word is wrong.

Message #43 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Kifer <michael.kifer <at> stonybrook.edu>
To: Dave Abrahams <dave <at> boostpro.com>
Cc: 11319-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Mon, 17 Dec 2012 18:28:55 -0500

[Message part 1 (text/html, inline)]

Message #46 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Dave Abrahams <dave <at> boostpro.com>
To: Michael Kifer <michael.kifer <at> stonybrook.edu>
Cc: 11319-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Mon, 17 Dec 2012 18:44:32 -0500

on Mon Dec 17 2012, Michael Kifer <michael.kifer-AT-stonybrook.edu> wrote:

> On 12/17/2012 06:22 PM, Dave Abrahams wrote:
>
>     on Fri Dec 14 2012, Michael Kifer <michael.kifer-AT-stonybrook.edu> wrote:
>     
>     
>         I think it is a good patch. One question: shouldn't the message
>         "Which buffer to compare to the merge buffer (A or B)? ")
>         be then changed to this?
>         "Which buffer to compare to the ancestor buffer (A or B)? ")
>         
>         This is not part of the  #11320 patch.
>         
>     I have no opinion about the change from "merge" to "ancestor."  It makes
>     absolutely no difference to me which of those words Emacs uses; I'll
>     interpret it exactly the same way.  I suggest you accept the patch as-is
>     and open a separate issue if you think the word is wrong.
>     
> The merge buffer is not the ancestor buffer, so the message would be
> wrong and confusing.
> We should change both or none.

I can't understand the idea that you can't correct the behavior without
also correcting the text.  They are separate issues related only by the
region of code in which they appear.  Again, I have no opinion about
whether the text should change, so if you're going to tie them together,
please change the text too.  The behavior is all that matters to me

-- 
Dave Abrahams
BoostPro Computing                  Software Development        Training
http://www.boostpro.com             Clang/LLVM/EDG Compilers  C++  Boost

Message #49 received at 11319-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Kifer <michael.kifer <at> stonybrook.edu>
To: Dave Abrahams <dave <at> boostpro.com>
Cc: 11319-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#11319: [PATCH 1/2] BUGFIX: select current-diff-comparison
	regions for ediff-inferior-compare-regions correctly
Date: Mon, 17 Dec 2012 19:01:53 -0500

[Message part 1 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.