* bug#1256: Race condition in vc-diff
@ 2008-10-25 22:33 Bob Rogers
2008-10-26 2:31 ` Stefan Monnier
2011-07-10 1:44 ` Chong Yidong
0 siblings, 2 replies; 4+ messages in thread
From: Bob Rogers @ 2008-10-25 22:33 UTC (permalink / raw)
To: emacs-pretest-bug
[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 1188 bytes --]
This problem seems to affect only the trunk, and not 22.3. (I last
built emacs on 21-Oct.)
To reproduce this, you need a working copy with a VCS that has a fast
backend "diff to base" command; something that doesn't have to contact a
server is probably a requirement (I used SVN with the WC on a local
disk). You also need a version-controlled file with a trivial change
with respect to the base version; touching a single line is sufficient.
If you visit this file and do "C-x v =" in a single-window frame that
is more than about 30 lines tall, and if the backend diff command is
fast enough, the diff window will remain half the size of the screen,
rather than being shrunk to fit the buffer. This is because
vc-exec-after finds that the buffer process has already finished, and
runs vc-diff-finish before the diff buffer has been made visible. In
that case, vc-diff-finish assumes that the user has already buried it.
I am not a big fan of this shrink-wrapping, but I do think vc-diff
ought to behave consistently (and it has worked this way for quite a
while now). The attached patch is sufficient to fix it.
-- Bob Rogers
http://rgrjr.dyndns.org/
[-- Attachment #2: Type: text/plain, Size: 934 bytes --]
--- lisp/vc.el.~1.706.~ 2008-10-06 21:02:25.000000000 -0400
+++ lisp/vc.el 2008-10-25 17:49:35.000000000 -0400
@@ -1507,10 +1507,12 @@
;; bindings are nicer for read only buffers. pcl-cvs does the
;; same thing.
(setq buffer-read-only t)
- (vc-exec-after `(vc-diff-finish ,(current-buffer) ',(when verbose
- messages)))
;; Display the buffer, but at the end because it can change point.
(pop-to-buffer (current-buffer))
+ ;; Do this after pop-to-buffer, because it needs to see the proper window
+ ;; state if it runs immediately. -- rgr, 25-Oct-08.
+ (vc-exec-after `(vc-diff-finish ,(current-buffer) ',(when verbose
+ messages)))
;; In the async case, we return t even if there are no differences
;; because we don't know that yet.
t)))
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#1256: Race condition in vc-diff
2008-10-25 22:33 bug#1256: Race condition in vc-diff Bob Rogers
@ 2008-10-26 2:31 ` Stefan Monnier
2008-10-26 3:45 ` Bob Rogers
2011-07-10 1:44 ` Chong Yidong
1 sibling, 1 reply; 4+ messages in thread
From: Stefan Monnier @ 2008-10-26 2:31 UTC (permalink / raw)
To: Bob Rogers; +Cc: emacs-pretest-bug, 1256
> If you visit this file and do "C-x v =" in a single-window frame that
> is more than about 30 lines tall, and if the backend diff command is
> fast enough, the diff window will remain half the size of the screen,
> rather than being shrunk to fit the buffer. This is because
> vc-exec-after finds that the buffer process has already finished, and
> runs vc-diff-finish before the diff buffer has been made visible. In
> that case, vc-diff-finish assumes that the user has already buried it.
> I am not a big fan of this shrink-wrapping, but I do think vc-diff
> ought to behave consistently (and it has worked this way for quite a
> while now). The attached patch is sufficient to fix it.
As the comment indicates, this patch is not quite good enough because
pop-to-buffer may change point. We probably will need to manipulate
point explicitly to work around the problem. BTW you can make the
"race-condition" deterministic by making the diff command synchronous
(as is the case with the RCS backend, for example, where your problem
should just *always* happen).
Stefan
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#1256: Race condition in vc-diff
2008-10-26 2:31 ` Stefan Monnier
@ 2008-10-26 3:45 ` Bob Rogers
0 siblings, 0 replies; 4+ messages in thread
From: Bob Rogers @ 2008-10-26 3:45 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 1256
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Sat, 25 Oct 2008 22:31:18 -0400
> If you visit this file and do "C-x v =" in a single-window frame that
> is more than about 30 lines tall, and if the backend diff command is
> fast enough, the diff window will remain half the size of the screen,
> rather than being shrunk to fit the buffer. This is because
> vc-exec-after finds that the buffer process has already finished, and
> runs vc-diff-finish before the diff buffer has been made visible. In
> that case, vc-diff-finish assumes that the user has already buried it.
> I am not a big fan of this shrink-wrapping, but I do think vc-diff
> ought to behave consistently (and it has worked this way for quite a
> while now). The attached patch is sufficient to fix it.
As the comment indicates, this patch is not quite good enough because
pop-to-buffer may change point. We probably will need to manipulate
point explicitly to work around the problem.
Problem? vc-diff-finish sets point in the diff buffer, so if it runs
immediately, it seems to me that it ought to override what pop-to-buffer
does. But perhaps I don't understand what point-changing behavior you
mean.
BTW you can make the "race-condition" deterministic by making the
diff command synchronous (as is the case with the RCS backend, for
example, where your problem should just *always* happen).
Stefan
It never occurred to me to try that. However, the same argument would
seem to apply for (setq vc-disable-async-diff t), but that only makes
the bug appear intermittently for me, even with the identical test case.
-- Bob
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#1256: Race condition in vc-diff
2008-10-25 22:33 bug#1256: Race condition in vc-diff Bob Rogers
2008-10-26 2:31 ` Stefan Monnier
@ 2011-07-10 1:44 ` Chong Yidong
1 sibling, 0 replies; 4+ messages in thread
From: Chong Yidong @ 2011-07-10 1:44 UTC (permalink / raw)
To: Bob Rogers; +Cc: 1256
> If you visit this file and do "C-x v =" in a single-window frame
> that is more than about 30 lines tall, and if the backend diff command
> is fast enough, the diff window will remain half the size of the
> screen, rather than being shrunk to fit the buffer. This is because
> vc-exec-after finds that the buffer process has already finished, and
> runs vc-diff-finish before the diff buffer has been made visible. In
> that case, vc-diff-finish assumes that the user has already buried it.
>
> I am not a big fan of this shrink-wrapping, but I do think vc-diff
> ought to behave consistently (and it has worked this way for quite a
> while now). The attached patch is sufficient to fix it.
I committed the patch to trunk. Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-07-10 1:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-25 22:33 bug#1256: Race condition in vc-diff Bob Rogers
2008-10-26 2:31 ` Stefan Monnier
2008-10-26 3:45 ` Bob Rogers
2011-07-10 1:44 ` Chong Yidong
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.