bug#37395: [PATCH v3] diff-mode.el: take into account patch separators

[Konstantin Kharlamov <hi-angel@yandex.ru>]

On Пн, окт 7, 2019 at 06:39, Lars Ingebrigtsen <larsi@gnus.org> 
wrote:
> (Some minor comments.)

Thanks!

> Konstantin Kharlamov <Hi-Angel@yandex.ru> writes:
> 
>>  +(defsubst diff-prev-line-if-patch-separator ()
>>  +  "Return previous line if it has patch separator produced by
>>  +git-format-patch."
> 
> I don't think this needs to be a defsubst -- a defun is fine here.

Will do.

> And the first doc string line should be a complete sentence.

Sorry, I'm not sure I get it: do you want me to keep the 
"git-format-patch." text on the first line, with the rest of the 
sentence?

>>  +(setq-local diff-buffer-type nil)
> 
> This should probably be a defvar and then a setq-local in `diff-mode'.

Will do.

>>  +  (save-excursion
>>  +    (if (re-search-forward "^diff --git" nil t)
>>  +        (setq diff-buffer-type 'git)
>>  +      (setq diff-buffer-type nil))))
> 
> Hm...  are we really guaranteed that all git diffs have that string in
> it somewhere?

Well, according to #git channel on Freenode and this answer 
https://stackoverflow.com/questions/39834729/what-does-the-diff-git-output-in-git-diff-refer-to 
apparently, it's there unless someone explicitly changed config for it 
to not be there.

But any other ideas to detect git format are welcome. I personally 
would prefer to not do detection at all, because I'm sure the case 
where we could misdetect text and miscalculate the diff header is too 
statistically insignificant. Too many things need to happen at once — 
and it doesn't seem that diff-mode being used by a lot of people too, 
since pretty major problem that I fix here went unnoticed for so long.