bug#25524: 26.0.50; diff-mode is broken

[Tino Calancha]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> On Wed, Jan 25, 2017 at 11:32 AM, Tino Calancha <tino.calancha@gmail.com> 
> wrote:
>>  ;; "index ", "old mode", "new mode", "new file mode" and
>> -;; "deleted file mode" are output by git-diff.
>> +;; "deleted file mode" are output by git-diff; "Index: " and
>> +;; "========..." by SVN.
>>  (defconst diff-file-junk-re
>> -  "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) 
>> mode\\|=== modified file")
>> +  (concat "Index: \\|" (make-string 67 ?=) "\\|"
>> +          "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) 
>> mode\\|=== modified file"))
>
> Is it safe to assume exactly 67 "="? Maybe it would be better to use
> something like "=\\{20,\\}", i.e. a sequence of 20 or more "=".
Humm probably 20 is OK, but i am not using SVN in a while: i ignore if
such '====...' might be coustomizable.
Updated the patch to use >=20 ?=.

Dima Kogan <dima@secretsauce.net> writes:

> More broadly, are we sure we need to be touching this? The previous
> approach and the one before that worked with the stock regexes.
1) The previous approach touch more things that just and already
existant regexp: it caused several problems already well discussed
in Bug#25105.

2) In the original approach, the mere existance of 'diff-file-junk-re'
as it is, it looks weird.  The file want to support all kind VCS,
not just Git,  so it's unnatural that the regexp just contains words
from Git diff headers.
Indeed, you can see that there are some hardcoded "^Index: " around
the file to handle SVN.
IMO, it's more readable adding the necessary stuff to
`diff-file-junk-re' and perform such operations uniformly just with
same function and regexp.  Then, if later turns out that we need more
keyword to handle other VCS we just need to change on line of code,
i.e., add those words into `diff-file-junk-re'.

That said, you are very welcome to improve current implementation so
that any `diff-file-junk-re' is required at all, without changing
other desirable functionality of the file.


;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>From c3788c73140af02d6fc2b414ddcdbaf273890cdf Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Thu, 26 Jan 2017 13:22:46 +0900
Subject: [PATCH] Fix Bug#25524

* lisp/vc/diff-mode.el (diff-beginning-of-hunk):
Return pos at beginning of hunk.
(diff-file-junk-re): Add SVN keywords.
---
 lisp/vc/diff-mode.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index b7ad8e8ebd..e609ca9f94 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -501,7 +501,8 @@ diff-end-of-hunk
 ;; "index ", "old mode", "new mode", "new file mode" and
 ;; "deleted file mode" are output by git-diff.
 (defconst diff-file-junk-re
-  "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== 
modified file")
+  (concat "Index: \\|=\\{20,\\}\\|" ; SVN
+          "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) 
mode\\|=== modified file"))
 
 ;; If point is in a diff header, then return beginning
 ;; of hunk position otherwise return nil.
@@ -545,7 +546,8 @@ diff-beginning-of-hunk
                (error "Can't find the beginning of the hunk")))
             ((re-search-backward regexp nil t)) ; In the middle of a hunk.
             ((re-search-forward regexp nil t) ; At first hunk header.
-             (forward-line 0))
+             (forward-line 0)
+             (point))
             (t (error "Can't find the beginning of the hunk"))))))
 
 (defun diff-unified-hunk-p ()
-- 
2.11.0

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.6)
 of 2017-01-26 built
Repository revision: 44765de2005fb56c5930383d6bd1e959a0102a45