bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

[Lluís Vilanova]

According to vc.el (when describing 'print-log'):

   If START-REVISION is given, then show the log starting from the
   revision.

Instead, the bazaar backend shows revisions since the beginning of the file's
history, up to the given revision.

To fix this, the line using 'start-revision' should read:

	    (when start-revision (list (format "-r%s.." start-revision)))

Instead of the current:

	    (when start-revision (list (format "-r..%s" start-revision)))


Thanks,
  Lluis

bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

[Eli Zaretskii]

> From: Lluís Vilanova <vilanova@ac.upc.edu>
> Date: Tue, 09 Apr 2013 21:17:35 +0200
> 
> According to vc.el (when describing 'print-log'):
> 
>    If START-REVISION is given, then show the log starting from the
>    revision.
> 
> Instead, the bazaar backend shows revisions since the beginning of the file's
> history, up to the given revision.

Are you sure this isn't a confusion about what is meant by "starting
from the revision"?

bug#14168: vc: [bzr] Properly limit revisions in 'vc-bzr-print-log'

[Lluís Vilanova]

Bazaar's log command supports the "--forward" argument, which can be set in
bazaar's configuration file (beyond emacs' knowledge; e.g. using the alias
"log = log --forward"). This explains why the current implementation is breaking
the declared generic semantics (by assuming "--forward" is not being used).

This can be fixed by changing it into:

            (if (and start-revision (= limit 1))
                (list (format "-c%s" start-revision))
              (append
               (when start-revision (list (format "-r%s.." start-revision)))
               (when limit (list "-l" (format "%s" limit)))))

Note that this will still break if both are given and limit is different from 1
(in which case "-c" cannot be used), but this is explicitly forbidden by the
generic semantics.


Thanks,
  Lluis

bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

[Lluís Vilanova]

Eli Zaretskii writes:

>> From: Lluís Vilanova <vilanova@ac.upc.edu>
>> Date: Tue, 09 Apr 2013 21:17:35 +0200
>> 
>> According to vc.el (when describing 'print-log'):
>> 
>> If START-REVISION is given, then show the log starting from the
>> revision.
>> 
>> Instead, the bazaar backend shows revisions since the beginning of the file's
>> history, up to the given revision.

> Are you sure this isn't a confusion about what is meant by "starting
> from the revision"?

Might well be. I understand it as "from given revision to newest
revision".

In any case, it breaks if the user adds the "--forward" argument. See my other
reply to the bug (which should be changed according to the meaning of "starting
from the revision").


Thanks,
  Lluis

bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

[Glenn Morris]

Lluís Vilanova wrote:

>> Are you sure this isn't a confusion about what is meant by "starting
>> from the revision"?
>
> Might well be. I understand it as "from given revision to newest
> revision".

This area is confusing. It is not well documented and inconsistently
implemented. The only documentation we have is in the commentary of
vc.el:

;;   If START-REVISION is given, then show the log starting from the
;;   revision.  At this point START-REVISION is only required to work
;;   in conjunction with LIMIT = 1.

This does not make it clear whether START-REVISION is the earliest or
latest revision. If we look at the implementations we see:

bzr  -r..start -l limit
    start is the latest revision
    show from newest to oldest
    show limit revisions

git  -n limit start
    start is the latest revision
    show from newest to oldest
    show limit revisions

hg   -rstart: -l limit
    start is the earliest revision
    show from oldest to newest
    show limit revisions

    for consistency with bzr and git, seems this should be -rstart:0  ?

svn  -rstart --limit limit
    show only single revision start
    limit is ignored

    for consistency with bzr and git, seems this should be -rstart:1 ?

mtn  --from start --last limit
    dunno what this does


In practice, the only place in Emacs that uses this is
vc-annotate-show-log-revision-at-line, which shows a single revision
only. So we have no way to tell what the intended behaviour is when
LIMIT != 1.

We should either document that it only works for LIMIT = 1, or decide
what it is supposed to do and make it consistent.
I'm guessing that START was supposed to be the newest revision, which is
the opposite of what I (like you) first thought.


I have no idea how to sensibly combine this with someone having "bzr log"
aliased to "bzr log --forward".

   bzr log --forward -r ..3 --limit 2 

will display revisions 1 and 2 rather than 2 and 3. There doesn't seem
to be a simple way to get the latter.

https://lists.ubuntu.com/archives/bazaar/2007q4/035828.html
   You are correct that the --limit option does not make much sense with
   the --forward option.  It is not meant to.
https://bugs.launchpad.net/bzr/+bug/320920

I don't know if we should bother trying to figure it out given that the
only caller of this uses LIMIT == 1. I'd probably just stick a
--no-aliases in there.

bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

[Lluís Vilanova]

Glenn Morris writes:

> Lluís Vilanova wrote:
>>> Are you sure this isn't a confusion about what is meant by "starting
>>> from the revision"?
>> 
>> Might well be. I understand it as "from given revision to newest
>> revision".

> This area is confusing. It is not well documented and inconsistently
> implemented. The only documentation we have is in the commentary of
> vc.el:
[...]
> In practice, the only place in Emacs that uses this is
> vc-annotate-show-log-revision-at-line, which shows a single revision
> only. So we have no way to tell what the intended behaviour is when
> LIMIT != 1.

> We should either document that it only works for LIMIT = 1, or decide
> what it is supposed to do and make it consistent.
> I'm guessing that START was supposed to be the newest revision, which is
> the opposite of what I (like you) first thought.


> I have no idea how to sensibly combine this with someone having "bzr log"
> aliased to "bzr log --forward".

>    bzr log --forward -r ..3 --limit 2 

> will display revisions 1 and 2 rather than 2 and 3. There doesn't seem
> to be a simple way to get the latter.

> https://lists.ubuntu.com/archives/bazaar/2007q4/035828.html
>    You are correct that the --limit option does not make much sense with
>    the --forward option.  It is not meant to.
> https://bugs.launchpad.net/bzr/+bug/320920

> I don't know if we should bother trying to figure it out given that the
> only caller of this uses LIMIT == 1. I'd probably just stick a
> --no-aliases in there.

Nice, I didn't know about the "--no-aliases" argument, and that might prove
useful. As a side note, I sent a small patch for the case where LIMIT is 1.

Looking at the code, I'd rather rename START-REVISION as REVISION, which I think
would be less confusing given the current semantics.

Lluis

bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set

[Glenn Morris]

Version: 24.4

Hopefully improved in trunk.

bug#14168: closed (Re: bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set)

[Lluís Vilanova]

Wouldn't this break when aliases are present and limit is 1? AFAIK, this is the
case in which I found out the "error".

bug#14168: closed (Re: bug#14168: 24.3.50; vc: Incorrect 'vc-bzr-print-log' when 'start-revision' and 'limit' are set)

[Glenn Morris]

Lluís Vilanova wrote:

> Wouldn't this break when aliases are present and limit is 1? AFAIK,
> this is the case in which I found out the "error".

Yeah, sorry, I realized that later. Will fix.