bug#5428: vc-annotate for Bzr does not show the date

bug#5428: vc-annotate for Bzr does not show the date

[Dan Nicolaescu]

The subject says it all, dates are not displayed by vc-annotate for bzr.

This is very bad, the date is very useful information.

It is available as a tooltip, but one cannot search for the information
in the tooltip.

What is worse is that this is inconsistent with what vc-annotate does
for all other backends, where the date is shown just fine.

`vc-bzr-annotate-command' goes through a lot of effort to reformat the
vc-annotate buffer with the end result of just hiding the date, and
changing the format of "author". 

The date is in a very compact format YYYYMMDD, so it does not take much
screen space.
If that is a problem we have the
vc-annotate-toggle-annotation-visibility command to hide all the extra
information.

vc-bzr-annotate-command should just limit itself to run the "bzr annotate"
command.  Just vc-bzr-annotate-time needs a minor adjustment to make
everything work.

bug#5428: vc-annotate for Bzr does not show the date

[Chong Yidong]

> The subject says it all, dates are not displayed by vc-annotate for bzr.
>
> This is very bad, the date is very useful information.
>
> It is available as a tooltip, but one cannot search for the information
> in the tooltip.
>
> What is worse is that this is inconsistent with what vc-annotate does
> for all other backends, where the date is shown just fine.

But what's displayed is exactly the same as "bzr annotate" on the tty.

I'd agree that vc-annotate should be consistent across backends, except
that vc-annotate with the SVN backend doesn't show the date either ("svn
annotate" on the tty also does not display the date).

bug#5428: vc-annotate for Bzr does not show the date

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > > The subject says it all, dates are not displayed by vc-annotate for bzr.
  > >
  > > This is very bad, the date is very useful information.
  > >
  > > It is available as a tooltip, but one cannot search for the information
  > > in the tooltip.
  > >
  > > What is worse is that this is inconsistent with what vc-annotate does
  > > for all other backends, where the date is shown just fine.
  > 
  > But what's displayed is exactly the same as "bzr annotate" on the tty.

The problem is that the command that is actually run is:

bzr annotate --long --all

If you run that from the command line you get the date.  You cannot get
it at all in vc-annotate.

And again this does not change the usefulness of seeing the date.


  > I'd agree that vc-annotate should be consistent across backends, except
  > that vc-annotate with the SVN backend doesn't show the date either ("svn
  > annotate" on the tty also does not display the date).

SVN has an incredibly dumb display for the date in the long form: 

    44 user 2009-10-14 20:48:43 -0700 (Wed, 14 Oct 2009) blah

Probably that's what motivated not to display the date.

Using svn as an argument is not the best idea.

bug#5428: vc-annotate for Bzr does not show the date

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Wed, 20 Jan 2010 11:36:20 -0500
> Cc: 5428@debbugs.gnu.org
> 
> But what's displayed is exactly the same as "bzr annotate" on the tty.

There's a switch to `annotate' that does display the date.

bug#5428: vc-annotate for Bzr does not show the date

[Stefan Monnier]

> The date is in a very compact format YYYYMMDD, so it does not take much
> screen space.

In vc-annotate, horizontal space is at a premium, since we're adding
"margin text" to a file who's content should already be expected to be
"using up all space" (e.g. all 80 columns).

I'd oppose adding the date until it can be configured away.


        Stefan

bug#5428: vc-annotate for Bzr does not show the date

[Dan Nicolaescu]

tags 5428 patch
quit

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > > The date is in a very compact format YYYYMMDD, so it does not take much
  > > screen space.
  > 
  > In vc-annotate, horizontal space is at a premium, since we're adding
  > "margin text" to a file who's content should already be expected to be
  > "using up all space" (e.g. all 80 columns).

Can you please explain how was this different with CVS (and all other
VCs besides svn)?

If space is so important why isn't hiding the whole annotation
vc-annotate-toggle-annotation-visibility even more useful?

  > I'd oppose adding the date until it can be configured away.

So bzr should be less functional that all other VCs until someone cares
to implement a new feature?

Meanwhile here's a patch for people that want to be able to fully use of
vc-annotate with bzr.  Just removes a bunch of code and adjusts 2 regexps.

Index: vc-bzr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-bzr.el,v
retrieving revision 1.94
diff -u -3 -p -u -p -r1.94 vc-bzr.el
--- vc-bzr.el	9 Dec 2009 06:04:12 -0000	1.94
+++ vc-bzr.el	21 Jan 2010 15:18:51 -0000
@@ -558,57 +585,25 @@ REV non-nil gets an error."
 Each line is tagged with the revision number, which has a `help-echo'
 property containing author and date information."
   (apply #'vc-bzr-command "annotate" buffer 'async file "--long" "--all"
-         (if revision (list "-r" revision)))
-  (lexical-let ((table (make-hash-table :test 'equal)))
-    (set-process-filter
-     (get-buffer-process buffer)
-     (lambda (proc string)
-       (when (process-buffer proc)
-         (with-current-buffer (process-buffer proc)
-           (setq string (concat (process-get proc :vc-left-over) string))
-           (while (string-match "^\\( *[0-9.]+ *\\) \\([^\n ]+\\) +\\([0-9]\\{8\\}\\)\\( |.*\n\\)" string)
-             (let* ((rev (match-string 1 string))
-                    (author (match-string 2 string))
-                    (date (match-string 3 string))
-                    (key (substring string (match-beginning 0)
-                                    (match-beginning 4)))
-                    (line (match-string 4 string))
-                    (tag (gethash key table))
-                    (inhibit-read-only t))
-               (setq string (substring string (match-end 0)))
-	       (unless tag
-		 (setq tag
-		       (propertize
-			(format "%s %-7.7s" rev author)
-			'help-echo (format "Revision: %d, author: %s, date: %s"
-					   (string-to-number rev)
-					   author date)
-			'mouse-face 'highlight))
-                 (puthash key tag table))
-               (goto-char (process-mark proc))
-               (insert tag line)
-               (move-marker (process-mark proc) (point))))
-           (process-put proc :vc-left-over string)))))))
+         (if revision (list "-r" revision))))
 
 (declare-function vc-annotate-convert-time "vc-annotate" (time))
 
 (defun vc-bzr-annotate-time ()
-  (when (re-search-forward "^ *[0-9.]+ +[^\n ]* +|" nil t)
-    (let ((prop (get-text-property (line-beginning-position) 'help-echo)))
-      (string-match "[0-9]+\\'" prop)
-      (let ((str (match-string-no-properties 0 prop)))
+  (when (re-search-forward "^[0-9.]+ +[^\n ]* +\\([0-9]\\{8\\}\\) |" nil t)
+    (let ((str (match-string-no-properties 1)))
       (vc-annotate-convert-time
        (encode-time 0 0 0
                       (string-to-number (substring str 6 8))
                       (string-to-number (substring str 4 6))
-                      (string-to-number (substring str 0 4))))))))
+                      (string-to-number (substring str 0 4)))))))
 
 (defun vc-bzr-annotate-extract-revision-at-line ()
   "Return revision for current line of annoation buffer, or nil.
 Return nil if current line isn't annotated."
   (save-excursion
     (beginning-of-line)
-    (if (looking-at "^ *\\([0-9.]+\\) +[^\n ]* +|")
+    (if (looking-at "^\\([0-9.]+\\) +[^\n ]* +\\([0-9]\\{8\\}\\) |")
         (match-string-no-properties 1))))
 
 (defun vc-bzr-command-discarding-stderr (command &rest args)

bug#5428: vc-annotate for Bzr does not show the date

[Stefan Monnier]

> Can you please explain how was this different with CVS (and all other
> VCs besides svn)?

It wasn't.  So it was a problem that wasn't solved for CVS.
Doesn't mean it should be re-introduced for Bzr.

> If space is so important why isn't hiding the whole annotation
> vc-annotate-toggle-annotation-visibility even more useful?

That's another option, although some info is useful to know which lines
were modified at the same time, and which lines where modified
before/after other lines (at least in my experience).

>> I'd oppose adding the date until it can be configured away.
> So bzr should be less functional that all other VCs until someone cares
> to implement a new feature?

I find it more functional, so your characterization is not helpful.


        Stefan

bug#5428: vc-annotate for Bzr does not show the date

[Dan Nicolaescu]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > > Can you please explain how was this different with CVS (and all other
  > > VCs besides svn)?
  > 
  > It wasn't.  So it was a problem that wasn't solved for CVS.
  > Doesn't mean it should be re-introduced for Bzr.

Does it mean that your personal preference should make it impossible to
do useful searches by date?

Why not fix it properly, instead of breaking it?  At least don't make it
strictly worse.

  > > If space is so important why isn't hiding the whole annotation
  > > vc-annotate-toggle-annotation-visibility even more useful?
  > 
  > That's another option, although some info is useful to know which lines
  > were modified at the same time, and which lines where modified
  > before/after other lines (at least in my experience).
  > 
  > >> I'd oppose adding the date until it can be configured away.
  > > So bzr should be less functional that all other VCs until someone cares
  > > to implement a new feature?
  > 
  > I find it more functional, so your characterization is not helpful.

Please explain why is it better not to be able to search by date.

bug#5428: vc-annotate for Bzr does not show the date

[Eli Zaretskii]

> Date: Fri, 22 Jan 2010 11:26:00 -0800 (PST)
> From: Dan Nicolaescu <dann@ics.uci.edu>
> Cc: 5428@debbugs.gnu.org
> 
> Please explain why is it better not to be able to search by date.

I don't use VC with Bazaar for now, but FWIW I agree with Dan here:
the dates are important.  I'm always annoyed when I forget to give
"bzr annotate" the switches that cause it to display the dates (gotta
define an alias for that one of these days), and am punished by
annotations that are next to useless.

bug#5428: vc-annotate for Bzr does not show the date

[Stefan Monnier]

>> > Can you please explain how was this different with CVS (and all other
>> > VCs besides svn)?
>> It wasn't.  So it was a problem that wasn't solved for CVS.
>> Doesn't mean it should be re-introduced for Bzr.
> Does it mean that your personal preference should make it impossible to
> do useful searches by date?

No, just that the current preference should not simply be replaced by
another one, unless we provide a way to configure VC to get the current
preference back.

FWIW, it's not impossible to do such searches: Use vc-print-log to see
which dates corresponds to which revision numbers, and then search by
revision number.

> Why not fix it properly, instead of breaking it?  At least don't make it
> strictly worse.

That's exactly what I want: to fix it properly, rather than to replace
one problem with another.

> Please explain why is it better not to be able to search by date.

I already explained it.


        Stefan

bug#5428: vc-annotate for Bzr does not show the date

[Dan Nicolaescu]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > >> > Can you please explain how was this different with CVS (and all other
  > >> > VCs besides svn)?
  > >> It wasn't.  So it was a problem that wasn't solved for CVS.
  > >> Doesn't mean it should be re-introduced for Bzr.
  > > Does it mean that your personal preference should make it impossible to
  > > do useful searches by date?
  > 
  > No, just that the current preference should not simply be replaced by
  > another one, unless we provide a way to configure VC to get the current
  > preference back.

Why not first implement whatever configurability YOU want, and then
change the default display so that it does not make work harder for
other people, instead of just breaking a long time working feature?

  > FWIW, it's not impossible to do such searches: Use vc-print-log to see
  > which dates corresponds to which revision numbers, and then search by
  > revision number.

How is this better than just searching in the vc-annotate buffer that
has been the normal work flow for years?


  > > Why not fix it properly, instead of breaking it?  At least don't make it
  > > strictly worse.
  > 
  > That's exactly what I want: to fix it properly, rather than to replace
  > one problem with another.

Can you please do it in such a way that you don't replace a working (and
in use for a very long time) setup with a broken one?

bug#5428: vc-annotate for Bzr does not show the date

[Glenn Morris]

Stefan Monnier wrote:

> In vc-annotate, horizontal space is at a premium, since we're adding
> "margin text" to a file who's content should already be expected to be
> "using up all space" (e.g. all 80 columns).

It's even worse for Git.

Using vc-annotate on a file under Git gives me a buffer where the first
90 columns are taken up with Git output. The actual file itself is
pushed completely off the right.

e95a67dc lisp/emacs-lisp/syntax.el (Stefan Monnier          2012-04-26
08:43:28  -0400   1)  ;;; syntax.el

By contrast in bzr only 21 columns are used.


The "lisp/emacs-lisp/syntax.el " bit seems useless to me.
The bzr version uses a truncated email address rather than a full name.
The Git date field seems way too specific.
The line numbers are totally unnecessary.

bug#5428: vc-annotate for Bzr does not show the date

[Dmitry Gutov]

On 05/09/2015 12:35 AM, Glenn Morris wrote:

> Using vc-annotate on a file under Git gives me a buffer where the first
> 90 columns are taken up with Git output. The actual file itself is
> pushed completely off the right.

I've made the date shorter, it'll save us 15 columns.

> The "lisp/emacs-lisp/syntax.el " bit seems useless to me.

That's only displayed if the file has been renamed at some point (some 
revisions are from different paths). But when it is, there's no way to 
turn it off.

Or rather there is: using the -c flag, but then it uses tabs in the 
output, and they make columns not always line up.

Maybe we could remove the file paths in a vc-run-delayed block. Should 
we worry that someone might find them useful?

> The bzr version uses a truncated email address rather than a full name.

We could replace the names with full email addresses, but those are even 
longer, at times.

> The line numbers are totally unnecessary.

Maybe it is, but it's a minor problem compared to the other fields. Also 
there's no way to turn it off.

bug#5428: vc-annotate for Bzr does not show the date

[Stefan Monnier]

FWIW, I've often wanted for vc-annotate to use a completely different
approach, where the buffer's text content is just the actual file's
content and the extra annotations are only displayed indirectly
(e.g. via a background color, tooltip text, margin text, or in-buffer
(but togglable by attribute)).

The way I see it working internally is that the process filter adds the
line annotations as a text-property to the file's lines, and then
a jit-lock-triggered function uses this text-property to change the
display as/when needed.

Also by using the unannotated file's text in buffer, we could enable the
normal font-lock rules, so we as to combine the normal font-lock
highlighting with the background-color of vc-annotate.


        Stefan

bug#5428: vc-annotate for Bzr does not show the date

[Dmitry Gutov]

On 05/17/2015 04:16 AM, Stefan Monnier wrote:
>
> FWIW, I've often wanted for vc-annotate to use a completely different
> approach, where the buffer's text content is just the actual file's
> content and the extra annotations are only displayed indirectly
> (e.g. via a background color, tooltip text, margin text, or in-buffer
> (but togglable by attribute)).

We already kind of have this possibility, though (via 
vc-annotate-toggle-annotation-visibility). It's not enough because dates 
and names are useful, and even having tooltips won't cut it.

The main stopper, I think, is the "togglable by attribute" UI (well, and 
also the implementations for extracting more granular data for each 
backend).

> Also by using the unannotated file's text in buffer, we could enable the
> normal font-lock rules, so we as to combine the normal font-lock
> highlighting with the background-color of vc-annotate.

That would be nice.

bug#5428: vc-annotate for Bzr does not show the date

[Dmitry Gutov]

On 05/17/2015 04:16 AM, Stefan Monnier wrote:

 > The way I see it working internally is that the process filter adds the
> line annotations as a text-property to the file's lines,

It should still be a different buffer though, mainly because after doing 
vc-annotate, I usually want to edit the file contents, and the 
annotation information won't stay up to date.

bug#5428: vc-annotate for Bzr does not show the date

[Stefan Monnier]

>> The way I see it working internally is that the process filter adds the
>> line annotations as a text-property to the file's lines,
> It should still be a different buffer though,

Obviously, yes.  Especially because the annotation is in general not
a bout the current version of the file.


        Stefan

bug#5428: vc-annotate for Bzr does not show the date

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 05/09/2015 12:35 AM, Glenn Morris wrote:
>
>> Using vc-annotate on a file under Git gives me a buffer where the first
>> 90 columns are taken up with Git output. The actual file itself is
>> pushed completely off the right.
>
> I've made the date shorter, it'll save us 15 columns.

It looks to me like the vc-annotate output is reasonable now, so I'm
closing this bug report.  If it's still an issue, please reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#5428: vc-annotate for Bzr does not show the date

[Glenn Morris]

Lars Ingebrigtsen wrote:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>> On 05/09/2015 12:35 AM, Glenn Morris wrote:
>>
>>> Using vc-annotate on a file under Git gives me a buffer where the first
>>> 90 columns are taken up with Git output. The actual file itself is
>>> pushed completely off the right.
>>
>> I've made the date shorter, it'll save us 15 columns.
>
> It looks to me like the vc-annotate output is reasonable now, so I'm
> closing this bug report.  If it's still an issue, please reopen.

The actual report was that vc-annotate for bzr does not show the date.
It still doesn't, so I don't see why you would think this is fixed.
The meta-issue was that the various backends display different
annotation information.

TLDR: reopening.

bug#5428: vc-annotate for Bzr does not show the date

[Lars Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

>> It looks to me like the vc-annotate output is reasonable now, so I'm
>> closing this bug report.  If it's still an issue, please reopen.
>
> The actual report was that vc-annotate for bzr does not show the date.
> It still doesn't, so I don't see why you would think this is fixed.
> The meta-issue was that the various backends display different
> annotation information.

Indeed.  The bzr output is really, really terse.

I've reworked Dan's patch to work apply now, and with that I get lines
like

1   larsi@gnus.org 20190627 | Foo

which seem nice and terse, but informative.  (The current git version,
on the other hand, is way overboard in noise.)

Does anybody still object to this change?  Stefan wanted this to be
customisable, and that would be nice, and should probably apply to all
backends, but it's a somewhat orthogonal issue...

diff --git a/lisp/vc/vc-bzr.el b/lisp/vc/vc-bzr.el
index ee1646cae5..89f1fcce37 100644
--- a/lisp/vc/vc-bzr.el
+++ b/lisp/vc/vc-bzr.el
@@ -869,61 +869,25 @@ vc-bzr-annotate-command
 property containing author and date information."
   (apply #'vc-bzr-command "annotate" buffer 'async file "--long" "--all"
          (append (vc-switches 'bzr 'annotate)
-		 (if revision (list "-r" revision))))
-  (let ((table (make-hash-table :test 'equal)))
-    (set-process-filter
-     (get-buffer-process buffer)
-     (lambda (proc string)
-       (when (process-buffer proc)
-         (with-current-buffer (process-buffer proc)
-           (setq string (concat (process-get proc :vc-left-over) string))
-           ;; Eg: 102020      Gnus developers          20101020 | regexp."
-           ;; As of bzr 2.2.2, no email address in whoami (which can
-           ;; lead to spaces in the author field) is allowed but discouraged.
-           ;; See bug#7792.
-           (while (string-match "^\\( *[0-9.]+ *\\) \\(.+?\\) +\\([0-9]\\{8\\}\\)\\( |.*\n\\)" string)
-             (let* ((rev (match-string 1 string))
-                    (author (match-string 2 string))
-                    (date (match-string 3 string))
-                    (key (substring string (match-beginning 0)
-                                    (match-beginning 4)))
-                    (line (match-string 4 string))
-                    (tag (gethash key table))
-                    (inhibit-read-only t))
-               (setq string (substring string (match-end 0)))
-	       (unless tag
-		 (setq tag
-		       (propertize
-			(format "%s %-7.7s" rev author)
-			'help-echo (format "Revision: %d, author: %s, date: %s"
-					   (string-to-number rev)
-					   author date)
-			'mouse-face 'highlight))
-                 (puthash key tag table))
-               (goto-char (process-mark proc))
-               (insert tag line)
-               (move-marker (process-mark proc) (point))))
-           (process-put proc :vc-left-over string)))))))
+		 (if revision (list "-r" revision)))))
 
 (declare-function vc-annotate-convert-time "vc-annotate" (&optional time))
 
 (defun vc-bzr-annotate-time ()
-  (when (re-search-forward "^ *[0-9.]+ +.+? +|" nil t)
-    (let ((prop (get-text-property (line-beginning-position) 'help-echo)))
-      (string-match "[0-9]+\\'" prop)
-      (let ((str (match-string-no-properties 0 prop)))
+  (when (re-search-forward "^[0-9.]+ +[^\n ]* +\\([0-9]\\{8\\}\\) |" nil t)
+    (let ((str (match-string-no-properties 1)))
       (vc-annotate-convert-time
        (encode-time 0 0 0
-                      (string-to-number (substring str 6 8))
-                      (string-to-number (substring str 4 6))
-                      (string-to-number (substring str 0 4))))))))
+                    (string-to-number (substring str 6 8))
+                    (string-to-number (substring str 4 6))
+                    (string-to-number (substring str 0 4)))))))
 
 (defun vc-bzr-annotate-extract-revision-at-line ()
   "Return revision for current line of annotation buffer, or nil.
 Return nil if current line isn't annotated."
   (save-excursion
     (beginning-of-line)
-    (if (looking-at "^ *\\([0-9.]+\\) +.* +|")
+    (if (looking-at "^\\([0-9.]+\\) +[^\n ]* +\\([0-9]\\{8\\}\\) |")
         (match-string-no-properties 1))))
 
 (defun vc-bzr-command-discarding-stderr (command &rest args)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#5428: vc-annotate for Bzr does not show the date

[Stefan Monnier]

> Does anybody still object to this change?  Stefan wanted this to be
> customisable, and that would be nice, and should probably apply to all
> backends, but it's a somewhat orthogonal issue...

Last time I used vc-annotate was in 2014, so the need for
customization is not very high up in my list.


        Stefan

bug#5428: vc-annotate for Bzr does not show the date

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Does anybody still object to this change?  Stefan wanted this to be
>> customisable, and that would be nice, and should probably apply to all
>> backends, but it's a somewhat orthogonal issue...
>
> Last time I used vc-annotate was in 2014, so the need for
> customization is not very high up in my list.

OK; I'll go ahead and apply the patch, because I think the resulting bzr
output was nicer...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#5428: vc-annotate for Bzr does not show the date

[Lars Ingebrigtsen]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> Meanwhile here's a patch for people that want to be able to fully use of
> vc-annotate with bzr.  Just removes a bunch of code and adjusts 2 regexps.

I've now applied this to the Emacs trunk.  Just took nine years.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no