Re: diff-apply-hunk documentation doesn't match implementation

[Stefan Monnier]

>> So, there was fundamentally only 1 file, right?  It just so happened that
>> diff-mode found 2 different matching files (one for the "old" and one for
>> the "new"), but it was unintended?

> Yes, that's one way to look at it.  In the real situation there were some
> differences between the orig tree and to working tree, but those were
> unrelated to the patch.

I'm not concerned about the content of those two files, but rather I'm
trying to understand why they were there for diff-mode to find.

E.g. was it just that the patch happened to use the same name for the old
tree as a tree you happened to have lying around?  Or was the name identical
precisely because that was the old tree you passed to `diff' to generate
the patch?

>> I'm not sure what you mean by "this behaviour".  I guess the confusing
>> behavior is mostly the inconsistency between diff-goto-source and
>> diff-apply-hook, is that right?

> Right, and it became worse after I read the doc string.

The doc is easy to fix ;-)
More seriously, I think I'll just revert my change and make the code follow
the doc for now.  The current behavior is indeed too confusing and fixing it
right will require more changes than I'd like for Emacs-22.

Thanks for bringing it up.


        Stefan


--- diff-mode.el        13 Mar 2007 10:29:22 -0400      1.98
+++ diff-mode.el        14 Mar 2007 11:13:12 -0400      
@@ -72,7 +72,7 @@
   :group 'diff-mode)
 
 (defcustom diff-jump-to-old-file nil
-  "*Non-nil means `diff-goto-source' jumps to the old file.
+  "Non-nil means `diff-goto-source' jumps to the old file.
 Else, it jumps to the new file."
   :type 'boolean
   :group 'diff-mode)
@@ -1280,7 +1280,7 @@
        (if (> (- (car forw) orig) (- orig (car back))) back forw)
       (or back forw))))
 
-(defsubst diff-xor (a b) (if a (not b) b))
+(defsubst diff-xor (a b) (if a (if (not b) a) b))
 
 (defun diff-find-source-location (&optional other-file reverse)
   "Find out (BUF LINE-OFFSET POS SRC DST SWITCHED).
@@ -1362,8 +1362,15 @@
 With a prefix argument, REVERSE the hunk."
   (interactive "P")
   (destructuring-bind (buf line-offset pos old new &optional switched)
-      ;; If REVERSE go to the new file, otherwise go to the old.
-      (diff-find-source-location (not reverse) reverse)
+      ;; Sometimes we'd like to have the following behavior: if REVERSE go
+      ;; to the new file, otherwise go to the old.  But that means that by
+      ;; default we use the old file, which is the opposite of the default
+      ;; for diff-goto-source, and is thus confusing.  Also when you don't
+      ;; know about it it's pretty surprising.
+      ;; TODO: make it possible to ask explicitly for this behavior.
+      ;; 
+      ;; This is duplicated in diff-test-hunk.
+      (diff-find-source-location nil reverse)
     (cond
      ((null line-offset)
       (error "Can't find the text to patch"))
@@ -1407,8 +1414,7 @@
 With a prefix argument, try to REVERSE the hunk."
   (interactive "P")
   (destructuring-bind (buf line-offset pos src dst &optional switched)
-      ;; If REVERSE go to the new file, otherwise go to the old.
-      (diff-find-source-location (not reverse) reverse)
+      (diff-find-source-location nil reverse)
     (set-window-point (display-buffer buf) (+ (car pos) (cdr src)))
     (diff-hunk-status-msg line-offset (diff-xor reverse switched) t)))