diff-mode: C-c C-c fails when location of hunk has been changed. C-d bound to backward-word.

unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed

* diff-mode: C-c C-c fails when location of hunk has been changed. C-d bound to backward-word.
@ 2008-05-26 14:04 Alan Mackenzie
  2008-05-26 15:55 ` Stefan Monnier
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Mackenzie @ 2008-05-26 14:04 UTC (permalink / raw)
  To: emacs-devel, Stefan Monnier

Hi, Stefan and Emacs!

GNU Emacs 23.0.60.1 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of
2008-04-22 on acm

In diff mode, whilst on a hunk, do C-c C-c.  This fails to find the
changed text in the target buffer, because it is now too far away from
where it was.  Fair enough.

Now edit the diff hunk so that the line number is now correct.  I.e.,
change:

*** 2078,2084 ****

to

*** 2128,2134 ****

.  Now do C-c C-c again.  Point is left at L2078 in the target buffer,
not L2128.  THIS IS A BUG.

#########################################################################

Whilst in the diff mode buffer, type C-d here:

*** 2078,2084 ****
           ^

The apparent effect of this is backward-word.  This is unfriendly.
(What actually happens is that an after-command hook recalculates 2084
and splats it into position.)

I think it would be better if diff-mode beeped with a message, e.g.

"End of hunk line-number cannot be edited".

-- 
Alan Mackenzie (Nuremberg, Germany).

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: diff-mode: C-c C-c fails when location of hunk has been changed. C-d bound to backward-word.
  2008-05-26 14:04 diff-mode: C-c C-c fails when location of hunk has been changed. C-d bound to backward-word Alan Mackenzie
@ 2008-05-26 15:55 ` Stefan Monnier
  2008-05-26 17:29   ` Stefan Monnier
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Monnier @ 2008-05-26 15:55 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

> In diff mode, whilst on a hunk, do C-c C-c.  This fails to find the
> changed text in the target buffer, because it is now too far away from
> where it was.  Fair enough.

Hmm... actually, it does an unbounded search, so it should find a place
if one exists.  It may not be the right one, of course.  Could you give
a detailed recipe to reproduce the problem?

> Now edit the diff hunk so that the line number is now correct.  I.e.,
> change:

> *** 2078,2084 ****

> to

> *** 2128,2134 ****

> .  Now do C-c C-c again.  Point is left at L2078 in the target buffer,
> not L2128.  THIS IS A BUG.

Could be.  Depends on a lot of things.  E.g. are the from&to file
names identical?  Is point in the `from' or the `to' part of the hunk?
Is the location of the hunk found?  What does the hunk look like?

> #########################################################################

> Whilst in the diff mode buffer, type C-d here:

> *** 2078,2084 ****
>            ^

> The apparent effect of this is backward-word.  This is unfriendly.
> (What actually happens is that an after-command hook recalculates 2084
> and splats it into position.)

> I think it would be better if diff-mode beeped with a message, e.g.

> "End of hunk line-number cannot be edited".

IIRC the after-command hook tries to stay out of the way if you touch
any of the headers.  Indeed, it seems to DTRT for unified hunks, but not
for context ones.  I'll take a look at it.

        Stefan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: diff-mode: C-c C-c fails when location of hunk has been changed. C-d bound to backward-word.
  2008-05-26 15:55 ` Stefan Monnier
@ 2008-05-26 17:29   ` Stefan Monnier
  0 siblings, 0 replies; 3+ messages in thread
From: Stefan Monnier @ 2008-05-26 17:29 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

>> Whilst in the diff mode buffer, type C-d here:

>> *** 2078,2084 ****
>> ^

>> The apparent effect of this is backward-word.  This is unfriendly.
>> (What actually happens is that an after-command hook recalculates 2084
>> and splats it into position.)

>> I think it would be better if diff-mode beeped with a message, e.g.

>> "End of hunk line-number cannot be edited".

> IIRC the after-command hook tries to stay out of the way if you touch
> any of the headers.  Indeed, it seems to DTRT for unified hunks, but not
> for context ones.  I'll take a look at it.

Actually it didn't do it for unified headers either (I just happened to
try and change a part that's not modified by the header-fixup code, duh!).
I've installed the patch below on the trunk which should fix
this problem.
Thanks,


        Stefan



--- diff-mode.el.~1.143.~	2008-05-16 00:14:33.000000000 -0400
+++ diff-mode.el	2008-05-26 13:23:04.000000000 -0400
@@ -369,6 +369,8 @@
 
 (defconst diff-hunk-header-re-unified
   "^@@ -\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? \\+\\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? @@")
+(defconst diff-context-mid-hunk-header-re
+  "--- \\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? ----$")
 
 (defvar diff-font-lock-keywords
   `((,(concat "\\(" diff-hunk-header-re-unified "\\)\\(.*\\)$")
@@ -376,7 +378,7 @@
     ("^\\(\\*\\{15\\}\\)\\(.*\\)$"                        ;context
      (1 diff-hunk-header-face) (2 diff-function-face))
     ("^\\*\\*\\* .+ \\*\\*\\*\\*". diff-hunk-header-face) ;context
-    ("^--- .+ ----$"             . diff-hunk-header-face) ;context
+    (,diff-context-mid-hunk-header-re . diff-hunk-header-face) ;context
     ("^[0-9,]+[acd][0-9,]+$"     . diff-hunk-header-face) ;normal
     ("^---$"                     . diff-hunk-header-face) ;normal
     ;; For file headers, accept files with spaces, but be careful to rule
@@ -977,7 +979,7 @@
                     (reversible t))
                 (replace-match "")
                 (unless (re-search-forward
-                         "^--- \\([0-9]+\\),\\(-?[0-9]+\\) ----$" nil t)
+                         diff-context-mid-hunk-header-re nil t)
                   (error "Can't find matching `--- n1,n2 ----' line"))
                 (let ((line2s (match-string 1))
                       (line2e (match-string 2))
@@ -1068,11 +1070,14 @@
 		  (when (= (char-after) ?-) (delete-char 1) (insert "+"))
 		  (forward-line 1))
 		(let ((half1 (delete-and-extract-region half1s (point))))
-		  (unless (looking-at "^--- \\([0-9]+,-?[0-9]+\\) ----$")
+		  (unless (looking-at diff-context-mid-hunk-header-re)
 		    (insert half1)
 		    (error "Can't find matching `--- n1,n2 ----' line"))
-		  (let ((str1 (match-string 1)))
-		    (replace-match lines1 nil nil nil 1)
+		  (let* ((str1end (or (match-end 2) (match-end 1)))
+                         (str1 (buffer-substring (match-beginning 1) str1end)))
+                    (goto-char str1end)
+                    (insert lines1)
+                    (delete-region (match-beginning 1) str1end)
 		    (forward-line 1)
 		    (let ((half2s (point)))
 		      (while (looking-at "[!+ \\][ \t]\\|#")
@@ -1137,7 +1142,7 @@
                 (if old1
                     (unless (string= new1 old1) (replace-match new1 t t nil 2))
                   (goto-char (match-end 2)) (insert "," new1))))
-	     ((looking-at "--- \\([0-9]+\\),\\([0-9]*\\) ----$")
+	     ((looking-at diff-context-mid-hunk-header-re)
 	      (when (> (+ space bang plus) 0)
 		(let* ((old1 (match-string 1))
 		       (old2 (match-string 2))
@@ -1189,25 +1194,29 @@
 	(goto-char (car diff-unhandled-changes))
 	;; Maybe we've cut the end of the hunk before point.
 	(if (and (bolp) (not (bobp))) (backward-char 1))
-	;; We used to fixup modifs on all the changes, but it turns out
-	;; that it's safer not to do it on big changes, for example
-	;; when yanking a big diff, since we might then screw up perfectly
-	;; correct values.  -stef
-	;; (unless (ignore-errors
-	;; 	  (diff-beginning-of-hunk)
-	;; 	  (save-excursion
-	;; 	    (diff-end-of-hunk)
-	;; 	    (> (point) (car diff-unhandled-changes))))
-	;;   (goto-char (car diff-unhandled-changes))
-	;; (re-search-forward diff-hunk-header-re (cdr diff-unhandled-changes))
-	;;   (diff-beginning-of-hunk))
-	;; (diff-fixup-modifs (point) (cdr diff-unhandled-changes))
+	;; We used to fixup modifs on all the changes, but it turns out that
+	;; it's safer not to do it on big changes, e.g. when yanking a big
+	;; diff, or when the user edits the header, since we might then
+	;; screw up perfectly correct values.  --Stef
 	(diff-beginning-of-hunk)
-	(when (save-excursion
+        (let* ((style (if (looking-at "\\*\\*\\*") 'context))
+               (start (line-beginning-position (if (eq style 'context) 3 2)))
+               (mid (if (eq style 'context)
+                        (save-excursion
+                          (re-search-forward diff-context-mid-hunk-header-re
+                                             nil t)))))
+          (when (and ;; Don't try to fixup changes in the hunk header.
+                 (> (car diff-unhandled-changes) start)
+                 ;; Don't try to fixup changes in the mid-hunk header either.
+                 (or (not mid)
+                     (< (cdr diff-unhandled-changes) (match-beginning 0))
+                     (> (car diff-unhandled-changes) (match-end 0)))
+                 (save-excursion
 		(diff-end-of-hunk nil 'donttrustheader)
-		(>= (point) (cdr diff-unhandled-changes)))
+                   ;; Don't try to fixup changes past the end of the hunk.
+                   (>= (point) (cdr diff-unhandled-changes))))
 	  (diff-fixup-modifs (point) (cdr diff-unhandled-changes)))))
-    (setq diff-unhandled-changes nil)))
+      (setq diff-unhandled-changes nil))))
 
 (defun diff-next-error (arg reset)
   ;; Select a window that displays the current buffer so that point
@@ -1367,7 +1376,7 @@
 	       (1+ (- (string-to-number (match-string 2))
 		      (string-to-number (match-string 1))))
 	     1))
-          (if (not (looking-at "--- \\([0-9]+\\)\\(?:,\\([0-9]+\\)\\)? ----$"))
+          (if (not (looking-at diff-context-mid-hunk-header-re))
               (error "Unrecognized context diff second hunk header format")
             (forward-line)
             (diff-sanity-check-context-hunk-half
@@ -1447,7 +1456,7 @@
 	     ;; context diff
 	     (forward-line 2)
 	     (setq src-pos (point))
-	     (re-search-forward "^--- " nil t)
+	     (re-search-forward diff-context-mid-hunk-header-re nil t)
 	     (forward-line 0)
 	     (setq divider-pos (point))
 	     (forward-line 1)
@@ -1563,7 +1572,8 @@
 		     (error "Can't find the hunk header")
 		   (if other (match-string 1)
 		     (if (match-end 3) (match-string 3)
-		       (unless (re-search-forward "^--- \\([0-9,]+\\)" nil t)
+		       (unless (re-search-forward
+                                diff-context-mid-hunk-header-re nil t)
 			 (error "Can't find the hunk separator"))
 		       (match-string 1)))))
 	   (file (or (diff-find-file-name other) (error "Can't find the file")))




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-05-26 17:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-26 14:04 diff-mode: C-c C-c fails when location of hunk has been changed. C-d bound to backward-word Alan Mackenzie
2008-05-26 15:55 ` Stefan Monnier
2008-05-26 17:29   ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).