GNU bug report logs - #35624
log-view-diff regression

Previous Next

Full log

Message #20 received at 35624 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>
Cc: 35624 <at> debbugs.gnu.org
Subject: Re: bug#35624: log-view-diff regression
Date: Thu, 9 May 2019 16:26:31 +0300

On 08.05.2019 22:52, Juri Linkov wrote:

> This is exactly what was my initial thought, but this is a wrong fix,
> as I realized later.

Okay, let's work on it.

I do want to just commit that patch first, since it was obviously the 
idea behind the previous change.

>> diff --git a/lisp/vc/log-view.el b/lisp/vc/log-view.el
>> index e47fad8908..e1e453115b 100644
>> --- a/lisp/vc/log-view.el
>> +++ b/lisp/vc/log-view.el
>> @@ -621,7 +621,8 @@ log-view-diff-common
>>                 (>= (point)
>>                     (save-excursion
>>                       (goto-char (car fr-entry))
>> -                    (forward-line))))
>> +                    (forward-line)
>> +                    (point))))
>>         (setq fr (vc-call-backend log-view-vc-backend 'previous-revision nil fr)))
>>       (vc-diff-internal
>>        t (list log-view-vc-backend
> 
> This patch doesn't check if the region's end is after the last revision,

What do you mean it doesn't? That's the whole purpose of the comparison 
(the part of the function the diff above changes).

> also fails if the summary line is expanded to multiline revision's header/body.

Isn't it the only situation where it fails?

I wouldn't say it's a hugely important case. The whole approach becomes 
iffy once the lower bound position is *inside* the revision entry.

Should it be the lower bound? Should the changes be included in the 
diff? I could never be sure without looking at the docstring.

>> Your proposal would fail in the presence of "Show 2X entries" (when the log
>> is long enough).
> 
> Yes, I know my previous patch is not perfect, I also tried
> 
>    (not (re-search-forward log-view-message-re nil t))
> 
> but it seems this is impossible to do because currently
> log-view.el doesn't support the notion of the end of
> the last revision expanded body.

The other option is check whether all lines between the point and EOB 
are either empty of start with "Show 2X entries". Which is a design I 
don't particularly like, but it could serve your goal.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.