Subject: bug#14989: closed (Re: bug#14989: 24.3.50; log-view-diff and log-view-diff-changeset default to different `to' revisions for git merge commits)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 31 Jul 2013 12:28:05 +0000

Your bug report

#14989: 24.3.50; log-view-diff and log-view-diff-changeset default to different `to' revisions for git merge commits

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
14989: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14989
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#14989: 24.3.50; log-view-diff and log-view-diff-changeset default to different `to' revisions for git merge commits
From: Dmitry Gutov <[email-masked]>
To: Stefan Monnier <[email-masked]>
Cc: [email-masked]
Date: Wed, 31 Jul 2013 15:26:45 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

Done!

On 31.07.2013 04:38, Stefan Monnier wrote:
>> This fix is simple enough. Apply?
>
> Looks good, thanks.
>
>> The functions could also use some further cleanup:
>> 1) Swap the `to' and `fr' local variables.
>
> I think that would make sense, yes.
>
>> 2) Extract the function bodies to one function with a third argument
>> (called `whole-changeset', maybe), and make them call it.
>
> If that can be done without too much extra code, that sounds good.

Subject: 24.3.50; log-view-diff and log-view-diff-changeset default to different `to' revisions for git merge commits
From: Dmitry Gutov <[email-masked]>
To: [email-masked]
Date: Wed, 31 Jul 2013 01:42:57 +0300
List-Id: <debbugs-submit.debbugs.gnu.org>

To reproduce:

Have a merge commit where the second branch (merged from) has a newer
last commit than the first branch (merged to).

`vc-print-root-log' buffer would look like this:

*  (HEAD, devel)b044fba..: Some Guy 2013-07-25 foo foo
*   2152d76..: Dmitry Gutov 2013-07-25 Merge branch 'bar' into devel
|\
| *  (bar)1554943..: Another Guy 2013-07-25 Abc
| * f92770a..: Another Guy 2013-07-25 Def
* | 5f5c068..: Another Guy 2013-07-25 Ghi

If I move to commit 2152d76 and press `d', it will show the diff against
1554943. If I press `D', if will show the diff against 5f5c068, which is
*the* parent commit, as far as Git is concerned (the first parent, to be
more exact), because it belongs to the branch `devel', and the merge was
done from `bar' to `devel'.

Since this is a `vc-print-root-log' buffer, both commands will show the
full changesets, not just changes in an individual file.

But the behavior in `vc-print-log' is the same in this respect, except
the diff `d' shows is just for the current file.

`log-view-diff' behavior makes certain amount of sense (it always diffs
against the revision on the line below), but it's inconsistent with
`log-view-diff-changeset', and it also backfires when the current commit
is not a child of the commit on the previous line. Example: f92770a and
5f5c068. As a result, we get a meaningless diff.

This fix is simple enough. Apply?

=== modified file 'lisp/vc/log-view.el'
--- lisp/vc/log-view.el 2013-06-11 06:36:06 +0000
+++ lisp/vc/log-view.el 2013-07-30 22:31:40 +0000
@@ -565,10 +565,7 @@
   (let ((fr (log-view-current-tag beg))
         (to (log-view-current-tag end)))
     (when (string-equal fr to)
-      (save-excursion
-        (goto-char end)
-        (log-view-msg-next)
-        (setq to (log-view-current-tag))))
+      (setq to (vc-call-backend log-view-vc-backend 'previous-revision nil fr)))
     (vc-diff-internal
      t (list log-view-vc-backend
             (if log-view-per-file-logs

The functions could also use some further cleanup:
1) Swap the `to' and `fr' local variables.
2) Extract the function bodies to one function with a third argument
(called `whole-changeset', maybe), and make them call it.

Objections?