From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: diff-apply-hunk documentation doesn't match implementation Date: Wed, 14 Mar 2007 11:13:46 -0400 Message-ID: References: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: sea.gmane.org 1173885261 20818 80.91.229.12 (14 Mar 2007 15:14:21 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Wed, 14 Mar 2007 15:14:21 +0000 (UTC) Cc: emacs-devel@gnu.org To: Andreas Schwab Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Mar 14 16:14:09 2007 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1HRVB2-0006dt-O0 for ged-emacs-devel@m.gmane.org; Wed, 14 Mar 2007 16:14:09 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HRVBw-00012H-4x for ged-emacs-devel@m.gmane.org; Wed, 14 Mar 2007 10:15:04 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1HRVBk-0000we-PK for emacs-devel@gnu.org; Wed, 14 Mar 2007 11:14:52 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1HRVBi-0000v2-CY for emacs-devel@gnu.org; Wed, 14 Mar 2007 11:14:51 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HRVBi-0000uv-4A for emacs-devel@gnu.org; Wed, 14 Mar 2007 10:14:50 -0500 Original-Received: from mercure.iro.umontreal.ca ([132.204.24.67]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1HRVAo-0006fT-09 for emacs-devel@gnu.org; Wed, 14 Mar 2007 11:13:54 -0400 Original-Received: from hidalgo.iro.umontreal.ca (hidalgo.iro.umontreal.ca [132.204.27.50]) by mercure.iro.umontreal.ca (Postfix) with ESMTP id 78E782CF2A2; Wed, 14 Mar 2007 11:13:53 -0400 (EDT) Original-Received: from faina.iro.umontreal.ca (faina.iro.umontreal.ca [132.204.26.177]) by hidalgo.iro.umontreal.ca (Postfix) with ESMTP id 201A23FE1; Wed, 14 Mar 2007 11:13:47 -0400 (EDT) Original-Received: by faina.iro.umontreal.ca (Postfix, from userid 20848) id 0A9736C188; Wed, 14 Mar 2007 11:13:46 -0400 (EDT) In-Reply-To: (Andreas Schwab's message of "Tue\, 13 Mar 2007 16\:45\:32 +0100") User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.93 (gnu/linux) X-DIRO-MailScanner-Information: Please contact the ISP for more information X-DIRO-MailScanner: Found to be clean X-DIRO-MailScanner-SpamCheck: n'est pas un polluriel, SpamAssassin (score=-2.82, requis 5, autolearn=not spam, ALL_TRUSTED -2.82) X-DIRO-MailScanner-From: monnier@iro.umontreal.ca X-detected-kernel: Linux 2.6 (newer, 3) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:67937 Archived-At: >> 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)))