Re: vc.el: asynchronous annotations

[David Kastrup]

Stefan Monnier <address@hidden> writes:

>> I have gone through with an example backend of my own as well as with
>> vc-cvs (which inexplicably still works synchronously here),
>
> Can you give me more information about your setup?  The CVS annotate is only
> asynchronous if the repository is remote and if vc-stay-local is t.
> Maybe your test repository is local?

Nope.  And vc-stay-local returns t.  No idea, I'll look more closely.

>> I am not sure I like the behavior when doing P or N or similar in the
>> annotation buffer: since the buffer is reused, it is cleared and
>> filled with new material while being displayed on the screen.
>
> It's not ideal, but there isn't much we can do about it.  The best is to
> provide an option for the user to choose something else (e.g. to force
> synchronous behavior).

It is extremely distracting.  One nuisance is that the correct line
can only be jumped to once the annotation is complete.  One could
possibly do this in the filter routine, switching from
insert-before-markers to insert once the correct line has been
reached, but I doubt it would be much prettier.

>> There are two ways around that, I guess.  One way would be to switch
>> back to the source buffer while the operation progresses.
>
> What do you mean "switch"?  As in `switch-to-buffer'?  That will fail if the
> window is dedicated.

Which is not necessarily the rule.  Anyway, that's not the solution I like.

>> Another way would be to _not_ reuse the buffer but instead delete the old
>> buffer once the new one is complete.
>
> Again, this will fail if the window is dedicated.
>
> OTOH it reminds me of a proposed feature: make it possible to exchange the
> text (plus related data like text-properties, markers, overlays) of
> two buffers.

That would not require much Lisp, I guess.  It would probably be
easiest to just append the new stuff at the end of the current buffer
in an invisible overlay, then move the point, delete the old material
and make the new stuff visible.  Perhaps some recentering will occur.

> With such a feature, we could indeed run the async process in
> another buffer and then bring the result into the old buffer.  The
> original motivation for such a feature was tar-mode where we'd like
> to move the tarball bytes into an auxiliary buffer (kept in unibyte
> mode) and have the "main" buffer only contain the tar listing (in
> multibyte mode).  This would save us from constantly switching
> between multibyte and unibyte modes and would clean up the code.

Shadow-buffers don't help in this case?

>> Anyway, here is a patch regarding the current working state at my end.
>> Stefan, what do you say?
>
>> *** vc.el    09 Jul 2007 23:10:04 +0200      1.432
>> --- vc.el    16 Jul 2007 01:09:39 +0200      
>> ***************
>> *** 3123,3129 ****
>>   use; you may override this using the second optional arg MODE."
>>     (interactive)
>>     (if mode (setq vc-annotate-display-mode mode))
>> !   (pop-to-buffer (or buffer (current-buffer)))
>>     (cond ((null vc-annotate-display-mode)
>>            ;; The ratio is global, thus relative to the global color-map.
>>            (kill-local-variable 'vc-annotate-color-map)
>> --- 3123,3129 ----
>>   use; you may override this using the second optional arg MODE."
>>     (interactive)
>>     (if mode (setq vc-annotate-display-mode mode))
>> !   (if buffer (set-buffer buffer))
>>     (cond ((null vc-annotate-display-mode)
>>            ;; The ratio is global, thus relative to the global color-map.
>>            (kill-local-variable 'vc-annotate-color-map)
>
> I strongly dislike this part.  The buffer should be shown immediately.
> Just as is the case when you do N and P (so any problem from which this may
> suffer has to be fixed anyway for other cases).
> Or are you saying that it is shown anyway because of the call to
> vc-annotate-display-select?

No, I've taken the pop-to-buffer out as you can see.  You are
basically proposing to exchange the one case where the behavior is
tolerable with intolerable behavior throughout, for consistency's
sake.  I'd rather like it the other way round...

>> --- 3192,3215 ----
>>            (rename-buffer temp-buffer-name t)
>>            ;; In case it had to be uniquified.
>>            (setq temp-buffer-name (buffer-name))))
>> !     (vc-call annotate-command file temp-buffer-name rev)
>> !     ;; we must setup the mode first, and then set our local
>> !     ;; variables before the show-function is called
>> !     (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)
>> !       (vc-annotate-display-select temp-buffer-name)
>   
> I'm not sure why you moved the vc-call outside of the
> with-output-to-temp-buffer.

I removed the with-output-to-temp-buffer altogether.  I think that the
motivation was that with-output-to-temp-buffer made it very obscure
for annotate-command to influence the encoding in which the command
output was interpreted from the annotate command, and in which it was
written out.

> It seems harmless, but it also seems pointless, so unless it's just
> an accident (resulting for other temporary attempts at something)
> I'd like to know why this was done.
>
> Similarly, what was the motivation for moving the call to
> vc-annotate-display-select?

vc-annotate-display-select is written in a manner that sets up the
highlighting immediately in some cases, and delays it with
vc-exec-after in some other cases.  Moving it made it do the right
thing in both synchronous and asynchronous calls.

>> !       (vc-exec-after
>> !        `(progn
>> !      (goto-line ,current-line ,temp-buffer-name)
>> !      (pop-to-buffer ,temp-buffer-name)
>> !      (unless (active-minibuffer-window)
>> !        (message "Annotating... done")))))))
>
> Doing a pop-to-buffer from a process sentinel is a problem: it
> works, but can be very annoying for the user (especially, but not
> only, if she's looking at her keyboard while typing).

Don't commands like man and woman also do that?

-- 
David Kastrup