emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Bugfix for ediff, two patches


From: Tom Breton (Tehom)
Subject: Bugfix for ediff, two patches
Date: Wed, 17 Nov 2010 14:35:41 -0500
User-agent: SquirrelMail/1.4.19

**** Explanation of the bug:

When told to "Compare currently highlighted difference regions",
ediff-inferior-compare-regions does *not* use the highlighted region as it
thinks it does.  It uses some other region that has nothing to do with
that.  This effectively defeats use of ediff for merging in git.

**** Instructions for reproducing it

I have attached files the demonstrate the bug.
 * Merge file.a.txt with file.b.txt using ancestor file.c.txt
   (Suggestion: use the code immediately below)
 * "n" to go to first clash
 * "a" to partly merge - just so it's merging nicely and not seeing
   "<<<<<<" ">>>>>>" "#####Ancestor" etc
 * "=" to start an inferior merge
 * "b" to compare to buffer B
 * Notice that ediff did not use the highlighted region like it said
   it would, but pulled some other text from B and Merge.

Here's code I found useful in seeing this bug and the fix for it, so I'm
including it here.  It just starts an merge-with-ancestor with the
respective files - saves time.  It works with the filenames I used for the
attachments that show the bug.

  (defun bug-ediff-merge-files-with-ancestor (dir)
     ""

     (interactive "DDirectory: ")
     (ediff-merge-files-with-ancestor
        (expand-file-name "file1.a.txt" dir)
        (expand-file-name "file1.b.txt" dir)
        (expand-file-name "file1.c.txt" dir)))

I also found it useful to give each set of {a,b,c} its own directory and
keep the filenames the same across directories.  I can't attach them to
this email with directory names, though.

**** Final diagnosis:

ediff-inferior-compare-regions was passing the wrong symbol to
ediff-clone-buffer-for-current-diff-comparison.  It assumed it was always
first 'A, then 'B.  This was accidentally correct for inferior comparisons
of 2-way comparisons, but wrong for inferior comparisons of other
comparisons.

**** Bugfix: Patch attached

I'm attaching a patch for this.

ediff-inferior-compare-regions has 3 branches for types of comparison that
it supports.  This fix seems to apply straightforwardly to all 3.  I have
exercised the fix in the "merge with ancestor" and "2-way" branches, but
not in the "3 way comparison" branch because I don't use it so I don't
have reasonable examples to try it on.

**** Related comments, for what they are worth:

 * There at least remain two bugs in ediff inferior comparisons, both much
less serious:

   * Wordwise merging doesn't handle whitespace reasonably when merging to
a blank element.  It just plops the non-blank element at the end of
whatever whitespace it ends up with, even across line breaks.  IMO it
would be more correct to infer the division of whitespace from the
whitespace around the non-blank element.  I've appended files that show
this behavior: The odd-whitespace and odd-whitespace-2 files, six in
all.

     It may not be worth the effort of coding it, though.  I don't plan to
fix this one.

   * Possibly related: Inferior comparisons wrongly include the line after
the comparison region.

     ediff-inferior-compare-regions deliberately gets whole lines.  Ie, if
comparison's end is somehow in the middle of a line,
ediff-inferior-compare-regions will advance it to the beginning of
the next line.  But it always assumes comparison's end is in the
middle of a line, so it always advances one line.

     I've attached a patch for this one too.

 * Some code in ediff could really use factoring.

**** Purpose of attachments

  * ediff-util.el.diff :: patch for main bug, from ediff-util.old.el to
ediff-util.fix.el

  * ediff-util.el.fix-line-bug.diff :: patch for extra-line bug, from
ediff-util.fix.el to ediff-util.fix2.el

       This patch is on top of the previous patch, because ISTM there's
little point in it if the more serious bug is not fixed.

  * Demo: the main bug, 3 files:

    * file1.{abc}.txt

  * Demo: the other behavior I think is wrong, 6 files:

    * odd-whitespace-2-file1.{abc}.txt

    * odd-whitespace-file1.{abc}.txt


       Tom Breton (Tehom)

Attachment: ediff-util.el.fix.diff
Description: Text Data

Attachment: ediff-util.el.fix-line-bug.diff
Description: Text Data

Attachment: file1.a.txt
Description: Text document

Attachment: file1.b.txt
Description: Text document

Attachment: file1.c.txt
Description: Text document

Attachment: odd-whitespace-file1.a.txt
Description: Text document

Attachment: odd-whitespace-file1.b.txt
Description: Text document

Attachment: odd-whitespace-file1.c.txt
Description: Text document

Attachment: odd-whitespace-2-file1.a.txt
Description: Text document

Attachment: odd-whitespace-2-file1.b.txt
Description: Text document

Attachment: odd-whitespace-2-file1.c.txt
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]