vc-annotate error

vc-annotate error

[JD Smith]

vc-annotate is giving me a similar error as was reported earlier for
vc.el revision 1.408:

 (file-error "Cannot open load file" "vc-nil")

This occurs because I have specified a default annotate mode of
'fullscale, and the `vc-annotate-display-autoscale' function still
relies on the variable `vc-annotate-backend' to call the appropriate
backend time functions I wrote years ago.  This was formerly a global
variable, but Stefan's changes to `vc-annotate' circa 1.408 made it buffer
local:

    (with-current-buffer temp-buffer-name
      (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
      (set (make-local-variable 'vc-annotate-parent-file) file)
      (set (make-local-variable 'vc-annotate-parent-rev) rev)
      (set (make-local-variable 'vc-annotate-parent-display-mode)
	   display-mode))

For whatever reason, these are not currently being set in the annotate
output buffer, which breaks the auto-scaling annotation display, since
it doesn't know the backend to use.  I haven't figured out how the
buffer-local variables are being lost.  Other annotate display modes
don't have backend-specific functions, so they continue to work.

By the way, should we make 'fullscale the default display mode?
Rather than scaling colors based on a fixed time window (1 year I
believe is the current default), it auto-scales to ensure the oldest
and newest colors both are used, to match the dynamic range of
annotation dates in the file to the available number of display
colors.  Currently, files less than a year old will be displayed
entirely in one color.

JD

Re: vc-annotate error

[Kevin Rodgers]

JD Smith wrote:
> vc-annotate is giving me a similar error as was reported earlier for
> vc.el revision 1.408:
> 
>  (file-error "Cannot open load file" "vc-nil")
> 
> This occurs because I have specified a default annotate mode of
> 'fullscale, and the `vc-annotate-display-autoscale' function still
> relies on the variable `vc-annotate-backend' to call the appropriate
> backend time functions I wrote years ago.  This was formerly a global
> variable, but Stefan's changes to `vc-annotate' circa 1.408 made it buffer
> local:
> 
>     (with-current-buffer temp-buffer-name
>       (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
>       (set (make-local-variable 'vc-annotate-parent-file) file)
>       (set (make-local-variable 'vc-annotate-parent-rev) rev)
>       (set (make-local-variable 'vc-annotate-parent-display-mode)
> 	   display-mode))

That snippet immediately follows this:

     (with-output-to-temp-buffer temp-buffer-name
       (vc-call annotate-command file (get-buffer temp-buffer-name) rev))

So could the problem be that those variables are set _after_ the
annotate buffer contents have been inserted and displayed (via
temp-buffer-show-function -> vc-annotate-display-select)?  Shouldn't
they be set before?

> For whatever reason, these are not currently being set in the annotate
> output buffer, which breaks the auto-scaling annotation display, since
> it doesn't know the backend to use.  I haven't figured out how the
> buffer-local variables are being lost.  Other annotate display modes
> don't have backend-specific functions, so they continue to work.

Are you sure they're not being set at all, vs. being set too late?

-- 
Kevin Rodgers

Re: vc-annotate error

[JD Smith]

On Fri, 07 Apr 2006 18:40:35 -0600, Kevin Rodgers wrote:

> JD Smith wrote:
>> vc-annotate is giving me a similar error as was reported earlier for
>> vc.el revision 1.408:
>> 
>>  (file-error "Cannot open load file" "vc-nil")
>> 
>> This occurs because I have specified a default annotate mode of
>> 'fullscale, and the `vc-annotate-display-autoscale' function still
>> relies on the variable `vc-annotate-backend' to call the appropriate
>> backend time functions I wrote years ago.  This was formerly a global
>> variable, but Stefan's changes to `vc-annotate' circa 1.408 made it
>> buffer local:
>> 
>>     (with-current-buffer temp-buffer-name
>>       (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
>>       (set (make-local-variable 'vc-annotate-parent-file) file) (set
>>       (make-local-variable 'vc-annotate-parent-rev) rev) (set
>>       (make-local-variable 'vc-annotate-parent-display-mode)
>> 	   display-mode))
> 
> That snippet immediately follows this:
> 
>      (with-output-to-temp-buffer temp-buffer-name
>        (vc-call annotate-command file (get-buffer temp-buffer-name) rev))
> 
> So could the problem be that those variables are set _after_ the annotate
> buffer contents have been inserted and displayed (via
> temp-buffer-show-function -> vc-annotate-display-select)?  Shouldn't they
> be set before?
> 
>> For whatever reason, these are not currently being set in the annotate
>> output buffer, which breaks the auto-scaling annotation display, since
>> it doesn't know the backend to use.  I haven't figured out how the
>> buffer-local variables are being lost.  Other annotate display modes
>> don't have backend-specific functions, so they continue to work.
> 
> Are you sure they're not being set at all, vs. being set too late?

I think that's probably correct, but after reversing the order I could
find no combination of buffer calls which would get the same buffer
behavior as currently, and also leave those local variables set.  I
believe the 'with-output-to-temp-buffer' body won't have access to
those values, since it doesn't actually set the current buffer to its
output buffer.  Given all the buffer switching that VC seems to do, it
may be somewhat unsafe to have these be buffer local.

Thanks,

JD

Re: vc-annotate error, PATCH

[JD Smith]

On Fri, 07 Apr 2006 16:28:16 -0700, JD Smith wrote:

> 
> vc-annotate is giving me a similar error as was reported earlier for vc.el
> revision 1.408:
> 
>  (file-error "Cannot open load file" "vc-nil")
> 
> This occurs because I have specified a default annotate mode of
> 'fullscale, and the `vc-annotate-display-autoscale' function still relies
> on the variable `vc-annotate-backend' to call the appropriate backend time
> functions I wrote years ago.  This was formerly a global variable, but
> Stefan's changes to `vc-annotate' circa 1.408 made it buffer local:
> 
>     (with-current-buffer temp-buffer-name
>       (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
>       (set (make-local-variable 'vc-annotate-parent-file) file) (set
>       (make-local-variable 'vc-annotate-parent-rev) rev) (set
>       (make-local-variable 'vc-annotate-parent-display-mode)
> 	   display-mode))
> 
> For whatever reason, these are not currently being set in the annotate
> output buffer, which breaks the auto-scaling annotation display, since it
> doesn't know the backend to use.  I haven't figured out how the
> buffer-local variables are being lost.  Other annotate display modes don't
> have backend-specific functions, so they continue to work.
> 
> By the way, should we make 'fullscale the default display mode? Rather
> than scaling colors based on a fixed time window (1 year I believe is the
> current default), it auto-scales to ensure the oldest and newest colors
> both are used, to match the dynamic range of annotation dates in the file
> to the available number of display colors.  Currently, files less than a
> year old will be displayed entirely in one color.

I looked into this further, and it turns out the ordering was correct,
but the `temp-buffer-show-function' was getting called before the
local variables were set, at the end of `with-output-to-temp-buffer'.
Since this in turns calls the auto-scaling display code, which
requires the local variables to be set, it fails.

Simply including the local variable setting inside the
`with-output-to-temp-buffer' body doesn't quite do it either, because
all local variables are being killed when vc-annotate-mode is entered
(as is normal for major modes).  The locals also get killed by
`with-output-to-temp-buffer', and indirectly by `vc-call'.  So the
only way to have local variables do this job is to set them after all
the commands which kill them off, but before the completion of
`with-output-to-temp-buffer'.  This is by no means obvious, and took a
fair bit of hunting to find all the things that were killing off
locals.

I also noticed that the default annotation colormap is sub-optimal.
The manual says "Text colored red is new, blue means old, and
intermediate colors indicate intermediate ages."  But the current
colormap jumps back and forth between green/magenta/yellow/etc, and
pure red is the 4th newest color, not the newest (currently an off
yellow). 

To address this, I created a new colormap with 18 entries which fixes
saturation at 70%, value at 100%, and rotates from red to blue in
equal angle ~14 degree increments of hue.  The saturation of 70% keeps
the text light enough to be readable on the default black background,
but still usable on a white background.  I also renamed the display
mode "Default" to "By Colormap", and made Oldest->Newest auto-scaling
the default choice.

Below is a patch which does all this.  Any objections to installing
it?  Give it a try on a really old file, like subr.el.  

JD


*** vc.el	10 Apr 2006 12:16:37 -0700	1.414
--- vc.el	10 Apr 2006 14:32:50 -0700	
***************
*** 584,592 ****
    :group 'vc
    :version "21.1")
  
! (defcustom vc-annotate-display-mode nil
    "Which mode to color the output of \\[vc-annotate] with by default."
!   :type '(choice (const :tag "Default" nil)
  		 (const :tag "Scale to Oldest" scale)
  		 (const :tag "Scale Oldest->Newest" fullscale)
  		 (number :tag "Specify Fractional Number of Days"
--- 584,592 ----
    :group 'vc
    :version "21.1")
  
! (defcustom vc-annotate-display-mode 'fullscale
    "Which mode to color the output of \\[vc-annotate] with by default."
!   :type '(choice (const :tag "By Colormap" nil)
  		 (const :tag "Scale to Oldest" scale)
  		 (const :tag "Scale Oldest->Newest" fullscale)
  		 (number :tag "Specify Fractional Number of Days"
***************
*** 617,646 ****
  
  ;; Annotate customization
  (defcustom vc-annotate-color-map
!   '(( 20. . "#FFCC00")
!     ( 40. . "#FF6666")
!     ( 60. . "#FF6600")
!     ( 80. . "#FF3300")
!     (100. . "#FF00FF")
!     (120. . "#FF0000")
!     (140. . "#CCCC00")
!     (160. . "#CC00CC")
!     (180. . "#BC8F8F")
!     (200. . "#99CC00")
!     (220. . "#999900")
!     (240. . "#7AC5CD")
!     (260. . "#66CC00")
!     (280. . "#33CC33")
!     (300. . "#00CCFF")
!     (320. . "#00CC99")
!     (340. . "#0099FF"))
    "Association list of age versus color, for \\[vc-annotate].
  Ages are given in units of fractional days.  Default is eighteen steps
! using a twenty day increment."
    :type 'alist
    :group 'vc)
  
! (defcustom vc-annotate-very-old-color "#0046FF"
    "Color for lines older than the current color range in \\[vc-annotate]]."
    :type 'string
    :group 'vc)
--- 617,647 ----
  
  ;; Annotate customization
  (defcustom vc-annotate-color-map
!   '(( 20. . "#FF4C4C")
!     ( 40. . "#FF764C")
!     ( 60. . "#FFA04C")
!     ( 80. . "#FFCA4C")
!     (100. . "#FFF44C")
!     (120. . "#DFFF4C")
!     (140. . "#B5FF4C")
!     (160. . "#8BFF4C")
!     (180. . "#61FF4C")
!     (200. . "#4CFF61")
!     (220. . "#4CFF8B")
!     (240. . "#4CFFB5")
!     (260. . "#4CFFDF")
!     (280. . "#4CF4FF")
!     (300. . "#4CCAFF")
!     (320. . "#4CA0FF")
!     (340. . "#4C76FF")
!     (360. . "#4C4CFF"))
    "Association list of age versus color, for \\[vc-annotate].
  Ages are given in units of fractional days.  Default is eighteen steps
! using a twenty day increment, from red to blue."
    :type 'alist
    :group 'vc)
  
! (defcustom vc-annotate-very-old-color "#4C4CFF"
    "Color for lines older than the current color range in \\[vc-annotate]]."
    :type 'string
    :group 'vc)
***************
*** 2971,2977 ****
  (easy-menu-define vc-annotate-mode-menu vc-annotate-mode-map
    "VC Annotate Display Menu"
    `("VC-Annotate"
!     ["Default" (unless (null vc-annotate-display-mode)
                   (setq vc-annotate-display-mode nil)
                   (vc-annotate-display-select))
       :style toggle :selected (null vc-annotate-display-mode)]
--- 2972,2978 ----
  (easy-menu-define vc-annotate-mode-menu vc-annotate-mode-map
    "VC Annotate Display Menu"
    `("VC-Annotate"
!     ["By Colormap" (unless (null vc-annotate-display-mode)
                   (setq vc-annotate-display-mode nil)
                   (vc-annotate-display-select))
       :style toggle :selected (null vc-annotate-display-mode)]
***************
*** 3016,3023 ****
    (when buffer
      (set-buffer buffer)
      (display-buffer buffer))
-   (if (not vc-annotate-parent-rev)
-       (vc-annotate-mode))
    (cond ((null vc-annotate-display-mode)
           ;; The ratio is global, thus relative to the global color-map.
           (kill-local-variable 'vc-annotate-color-map)
--- 3017,3022 ----
***************
*** 3087,3101 ****
  	      ;; In case it had to be uniquified.
  	      (setq temp-buffer-name (buffer-name))))
      (with-output-to-temp-buffer temp-buffer-name
!       (vc-call annotate-command file (get-buffer temp-buffer-name) rev))
!     (with-current-buffer temp-buffer-name
!       (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
!       (set (make-local-variable 'vc-annotate-parent-file) file)
!       (set (make-local-variable 'vc-annotate-parent-rev) rev)
!       (set (make-local-variable 'vc-annotate-parent-display-mode)
! 	   display-mode))
! 
!   (message "Annotating... done")))
  
  (defun vc-annotate-prev-version (prefix)
    "Visit the annotation of the version previous to this one.
--- 3086,3104 ----
  	      ;; In case it had to be uniquified.
  	      (setq temp-buffer-name (buffer-name))))
      (with-output-to-temp-buffer temp-buffer-name
!       (vc-call annotate-command file (get-buffer temp-buffer-name) rev)
!       ;; we must setup the mode first, and then set our local
!       ;; variables before the show-function is called at the exit of
!       ;; with-output-to-temp-buffer
!       (with-current-buffer temp-buffer-name
!         (if (not (equal major-mode 'vc-annotate-mode))
!             (vc-annotate-mode))
!         (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
!         (set (make-local-variable 'vc-annotate-parent-file) file)
!         (set (make-local-variable 'vc-annotate-parent-rev) rev)
!         (set (make-local-variable 'vc-annotate-parent-display-mode)
!              display-mode)))
!     (message "Annotating... done")))
  
  (defun vc-annotate-prev-version (prefix)
    "Visit the annotation of the version previous to this one.

Re: vc-annotate error, PATCH

[Stefan Monnier]

> I looked into this further, and it turns out the ordering was correct,
> but the `temp-buffer-show-function' was getting called before the
> local variables were set, at the end of `with-output-to-temp-buffer'.
> Since this in turns calls the auto-scaling display code, which
> requires the local variables to be set, it fails.
[...]
> Below is a patch which does all this.  Any objections to installing
> it?  Give it a try on a really old file, like subr.el.

Thanks.  The bug-fix part of the patch looks fine to me, thank you for
tracking it down and please install it.
I have no opinion on the colormap part of the code.


        Stefan

Re: vc-annotate error, PATCH

[JD Smith]

On Mon, 10 Apr 2006 18:27:46 -0400, Stefan Monnier wrote:

>> I looked into this further, and it turns out the ordering was correct,
>> but the `temp-buffer-show-function' was getting called before the
>> local variables were set, at the end of `with-output-to-temp-buffer'.
>> Since this in turns calls the auto-scaling display code, which
>> requires the local variables to be set, it fails.
> [...]
>> Below is a patch which does all this.  Any objections to installing
>> it?  Give it a try on a really old file, like subr.el.
> 
> Thanks.  The bug-fix part of the patch looks fine to me, thank you for
> tracking it down and please install it.
> I have no opinion on the colormap part of the code.

No problem.  Unless anyone has any objections in the next day or so to
the changes in colormap and default display mode, I'll install.

JD

Re: vc-annotate error, PATCH

[Richard Stallman]

    To address this, I created a new colormap with 18 entries which fixes
    saturation at 70%, value at 100%, and rotates from red to blue in
    equal angle ~14 degree increments of hue.  The saturation of 70% keeps
    the text light enough to be readable on the default black background,
    but still usable on a white background.

How well does it work on a tty?

Re: vc-annotate error, PATCH

[JD Smith]

On Tue, 11 Apr 2006 12:57:23 -0400, Richard Stallman wrote:

>     To address this, I created a new colormap with 18 entries which fixes
>     saturation at 70%, value at 100%, and rotates from red to blue in
>     equal angle ~14 degree increments of hue.  The saturation of 70% keeps
>     the text light enough to be readable on the default black background,
>     but still usable on a white background.
> 
> How well does it work on a tty?

I hadn't thought of that, but tried and it does a reasonable job.
Less dynamic range in the yellow-green regime, but that is expected
given the small number of colors TTY's support. Here is a side by side
screen shot comparing the X11 vs. terminal annotations of the same
file:

 http://turtle.as.arizona.edu/emacs/annot_color.png

The only drawback is that green doesn't seem to get used much, with
yellow showing up instead for many colors.  I believe this is a
function of how substitute color are approximated, and the 70%
compromise saturation I use (although increasing this doesn't really
help that much).

Is there a canonical set of RGB values that terminals typically can
display?  Perhaps I could tune the map to spread out among the 5
colors more, but at some point you start sacrificing readibility on
full color displays, and over-relying on the algorithm for color
substitution.  Another option: is it possible or desirable to have a
separate color map for TTYs?  I could envision just using all the TTY
colors except the background color, in the order they are presented
from red to blue (now all but magenta are used).  May be more trouble
than it's worth.

JD

Re: vc-annotate error, PATCH

[Dan Nicolaescu]

JD Smith <jdsmith@as.arizona.edu> writes:

  > On Tue, 11 Apr 2006 12:57:23 -0400, Richard Stallman wrote:
  > 
  > >     To address this, I created a new colormap with 18 entries which fixes
  > >     saturation at 70%, value at 100%, and rotates from red to blue in
  > >     equal angle ~14 degree increments of hue.  The saturation of 70% keeps
  > >     the text light enough to be readable on the default black background,
  > >     but still usable on a white background.
  > > 
  > > How well does it work on a tty?
  > 
  > I hadn't thought of that, but tried and it does a reasonable job.
  > Less dynamic range in the yellow-green regime, but that is expected
  > given the small number of colors TTY's support. Here is a side by side
  > screen shot comparing the X11 vs. terminal annotations of the same
  > file:
  > 
  >  http://turtle.as.arizona.edu/emacs/annot_color.png
  > 
  > The only drawback is that green doesn't seem to get used much, with
  > yellow showing up instead for many colors.  I believe this is a
  > function of how substitute color are approximated, and the 70%
  > compromise saturation I use (although increasing this doesn't really
  > help that much).
  > 
  > Is there a canonical set of RGB values that terminals typically can
  > display?  Perhaps I could tune the map to spread out among the 5
  > colors more, but at some point you start sacrificing readibility on
  > full color displays, and over-relying on the algorithm for color
  > substitution.  Another option: is it possible or desirable to have a
  > separate color map for TTYs?  I could envision just using all the TTY
  > colors except the background color, in the order they are presented
  > from red to blue (now all but magenta are used).  May be more trouble
  > than it's worth.

If you change anything, please don't change it based on the display
being a TTY, but on the number of colors the TTY supports (see
display-color-cells and the min-color attribute for defface).

Your patch works very well on terminals that support 256 colors.
(Of the terminals in wide use only the Linux console and KDE's konsole
do not support 256 colors. xterm, rxvt, Gnome terminal and putty
support 256 colors.)

Re: vc-annotate error, PATCH

[JD Smith]

On Tue, 11 Apr 2006 11:34:20 -0700, Dan Nicolaescu wrote:

>   > I could envision just using all the TTY
>   > colors except the background color, in the order they are presented
>   > from red to blue (now all but magenta are used).  May be more trouble
>   > than it's worth.
> 
> If you change anything, please don't change it based on the display
> being a TTY, but on the number of colors the TTY supports (see
> display-color-cells and the min-color attribute for defface).
> 
> Your patch works very well on terminals that support 256 colors.
> (Of the terminals in wide use only the Linux console and KDE's konsole
> do not support 256 colors. xterm, rxvt, Gnome terminal and putty
> support 256 colors.)

OK, thanks.  The screen shot was from Gnome-terminal.  I can't seem to
enable 256 color support, and display-color-cells is 8.  Must be a
compile-time option which isn't commonly used.  Can you send me your
(tty-color-alist) output from such a terminal offline?

I have a patch now that just sorts the non-white and non-black colors
in a TTY to put red first, blue last, and whatever in between, and
make that the colormap.  I could enable this if on a TTY and
display-color-cells<=8.  It would not be the same "color theme" but it
would have much better dynamic range and color usage for 8 color TTYs.
Further thoughts welcome.

JD

Re: vc-annotate error, PATCH

[Dan Nicolaescu]

JD Smith <jdsmith@as.arizona.edu> writes:

  > On Tue, 11 Apr 2006 11:34:20 -0700, Dan Nicolaescu wrote:
  > 
  > >   > I could envision just using all the TTY
  > >   > colors except the background color, in the order they are presented
  > >   > from red to blue (now all but magenta are used).  May be more trouble
  > >   > than it's worth.
  > > 
  > > If you change anything, please don't change it based on the display
  > > being a TTY, but on the number of colors the TTY supports (see
  > > display-color-cells and the min-color attribute for defface).
  > > 
  > > Your patch works very well on terminals that support 256 colors.
  > > (Of the terminals in wide use only the Linux console and KDE's konsole
  > > do not support 256 colors. xterm, rxvt, Gnome terminal and putty
  > > support 256 colors.)
  > 
  > OK, thanks.  The screen shot was from Gnome-terminal.  I can't seem to
  > enable 256 color support, and display-color-cells is 8.  Must be a
  > compile-time option which isn't commonly used.  

I think it's currently just in CVS, it will be in the next version,
and it will be enabled by default.

  > Can you send me your (tty-color-alist) output from such a terminal
  > offline?

Will do.

Re: vc-annotate error, PATCH

[Richard Stallman]

    I have a patch now that just sorts the non-white and non-black colors
    in a TTY to put red first, blue last, and whatever in between, and
    make that the colormap.  I could enable this if on a TTY and
    display-color-cells<=8.  It would not be the same "color theme" but it
    would have much better dynamic range and color usage for 8 color TTYs.
    Further thoughts welcome.

Sounds like the right thing.

Re: vc-annotate error, PATCH

[JD Smith]

On Wed, 12 Apr 2006 13:09:45 -0400, Richard Stallman wrote:

>     I have a patch now that just sorts the non-white and non-black
>     colors in a TTY to put red first, blue last, and whatever in
>     between, and make that the colormap.  I could enable this if on a
>     TTY and display-color-cells<=8.  It would not be the same "color
>     theme" but it would have much better dynamic range and color usage
>     for 8 color TTYs. Further thoughts welcome.
> 
> Sounds like the right thing.

I've updated CVS with this patch (and the other bug fixes mentioned). I
ended up going with the order red,yellow,cyan,green,magenta,blue from
newest to oldest, since that at least crudely approximates the color
direction for the proper hue wheel map (magenta aside).  I use this
special map only for 8 color TTY devices.  I went to some trouble to allow
for default colors which don't have these standard names, since I wasn't
sure if they were the same across all terminals.

Dan Nicolaescu gave me the hint to try:

 set TERM=xterm-256color

in recent xterms.  Emacs in terminal mode there looks fantastic, and the
new rotating hue colormap does very well.  Here's hoping all terminals
quickly migrate to 256 colors.