23.0.50; vc-diff needs CVS repository now

23.0.50; vc-diff needs CVS repository now

[Nick Roberts]

I have a copy of an unmodified file in CVS (varobj.c.~1.96~, say) and
vc-cvs-stay-local and vc-stay-local are t.  If I do vc-diff (V-x v =) in
the buffer with varobj.c (CVS:1.96 in mode-line), Emacs now tries to access the
CVS repository to compute the diff buffer, rather than using varobj.c.~1.96~.


In GNU Emacs 23.0.50.34 (i686-pc-linux-gnu, GTK+ Version 2.10.11)
 of 2007-10-25 on kahikatea.snap.net.nz
Windowing system distributor `The X.Org Foundation', version 11.0.70200000
configured using `configure  'CFLAGS=-g3''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_NZ.UTF-8
  locale-coding-system: utf-8
  default-enable-multibyte-characters: t

Major mode: VM Summary

Minor modes in effect:
  show-paren-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<prior> <prior> <prior> <down-mouse-1> <mouse-1> C-s 
C-w C-w C-s C-s C-s <f5> <f5> <f5> <f5> <f5> <f5> <f5> 
<f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> 
<f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> 
<f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> <f5> 
<f5> <f5> <f5> <f5> <f5> <f5> <f6> <f6> <f6> <f6> <f6> 
<f6> <f6> <switch-frame> <down-mouse-1> <mouse-movement> 
<mouse-1> <help-echo> <switch-frame> <down-mouse-1> 
<drag-mouse-1> <down-mouse-1> <mouse-movement> <drag-mouse-1> 
<end> <down-mouse-2> <mouse-2> <down-mouse-1> <mouse-1> 
<next> <next> <next> <next> <next> <next> <next> <next> 
<prior> <help-echo> <down-mouse-1> <mouse-movement> 
<mouse-1> C-s C-w C-w C-w C-w C-w C-w C-w <help-echo> 
<switch-frame> <down-mouse-1> <mouse-movement> <mouse-1> 
<help-echo> <down-mouse-1> <mouse-movement> <drag-mouse-1> 
C-x C-b <help-echo> <help-echo> <down-mouse-2> <mouse-2> 
<down-mouse-1> <mouse-1> C-s C-s C-r <switch-frame> 
<down-mouse-1> <mouse-1> <help-echo> <switch-frame> 
<down-mouse-1> <mouse-1> <help-echo> <switch-frame> 
<down-mouse-1> <mouse-1> <help-echo> <help-echo> <switch-frame> 
<down-mouse-1> <mouse-1> <help-echo> <help-echo> <down-mouse-1> 
<mouse-movement> <mouse-1> C-x C-v C-g <down-mouse-1> 
<mouse-movement> <mouse-1> C-x C-b <help-echo> <help-echo> 
<help-echo> <down-mouse-2> <mouse-2> <help-echo> <down-mouse-1> 
<mouse-1> <help-echo> C-s C-w C-w C-w C-w C-w <down-mouse-1> 
<mouse-1> C-s C-s C-r <down-mouse-1> <mouse-movement> 
<mouse-movement> <drag-mouse-1> C-w <down-mouse-1> 
<mouse-movement> <mouse-movement> <drag-mouse-1> C-w 
<down-mouse-2> <mouse-2> <down-mouse-1> <mouse-1> <tab> 
<down-mouse-1> <mouse-movement> <mouse-movement> <drag-mouse-1> 
C-w <down-mouse-1> <drag-mouse-1> <down-mouse-1> <mouse-movement> 
<mouse-movement> <drag-mouse-1> C-w <return> C-x C-s 
<down-mouse-1> <mouse-1> <tab> <down-mouse-1> <mouse-1> 
<tab> C-x C-s <help-echo> <down-mouse-1> <mouse-movement> 
<mouse-movement> <mouse-1> <help-echo> <help-echo> 
<switch-frame> <down-mouse-1> <mouse-movement> <mouse-1> 
<down-mouse-1> <mouse-1> <f5> <f5> <f5> <switch-frame> 
<down-mouse-1> <mouse-1> <help-echo> <help-echo> <help-echo> 
<down-mouse-1> <mouse-movement> <mouse-1> <help-echo> 
<down-mouse-1> <mouse-1> <help-echo> <down-mouse-1> 
<mouse-1> C-x C-b <help-echo> <help-echo> <help-echo> 
<down-mouse-2> <mouse-2> C-x v = <help-echo> <help-echo> 
<help-echo> <help-echo> <down-mouse-2> <mouse-2> <help-echo> 
<down-mouse-1> <mouse-1> <down-mouse-1> <mouse-movement> 
<mouse-1> <down-mouse-1> <mouse-1> C-h v c <backspace> 
v c SPC c v s SPC s t a SPC SPC <return> <help-echo> 
<switch-frame> <down-mouse-1> <mouse-1> M-x r e p o 
p <backspace> r t SPC e m a SPC SPC <return>

Recent messages:
Mark saved where search started [2 times]
Quit
Mark saved where search started [2 times]
Mark set
Saving file /home/nickrob/src/gdb/varobj.c...
Wrote /home/nickrob/src/gdb/varobj.c
Saving file /home/nickrob/src/gdb/varobj.c...
Wrote /home/nickrob/src/gdb/varobj.c
Finding changes in /home/nickrob/src/gdb/testsuite/gdb.mi/mi-var-cmd.exp...
Type C-x 1 to remove help window.  
C-x v = runs the command vc-diff

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: 23.0.50; vc-diff needs CVS repository now

[Glenn Morris]

Any comment on this? The part of vc-diff-internal that does a local
diff where possible seems to have disappeared in the 2007-10-10 change
"rewrite for filesets".

Nick Roberts wrote:

> I have a copy of an unmodified file in CVS (varobj.c.~1.96~, say)
> and vc-cvs-stay-local and vc-stay-local are t. If I do vc-diff (V-x
> v =) in the buffer with varobj.c (CVS:1.96 in mode-line), Emacs now
> tries to access the CVS repository to compute the diff buffer,
> rather than using varobj.c.~1.96~.
>
>
> In GNU Emacs 23.0.50.34 (i686-pc-linux-gnu, GTK+ Version 2.10.11)
>  of 2007-10-25 on kahikatea.snap.net.nz

Re: 23.0.50; vc-diff needs CVS repository now

[Eric S. Raymond]

Glenn Morris <rgm@gnu.org>:
> Any comment on this? The part of vc-diff-internal that does a local
> diff where possible seems to have disappeared in the 2007-10-10 change
> "rewrite for filesets".

That's right, because the front end was the wrong place to do it.  
Modern VCS diff tools automatically stay local when they can.
Subversion diff does the right thing, and it's not even an issue
in the 3G systems (they keep local repositories).  It's the
kind of variation that should be handled in the VCS-specific back ends.

Actually CVS is the only awkward case.  So what ought to happen is
for something like the old stay-local logic to be implemented in the CVS 
back end.

This is one of my two remaining to-do items for the mode.  But someone
who is more of a CVS expert than I might get it done faster.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: 23.0.50; vc-diff needs CVS repository now

[Nick Roberts]

 > This is one of my two remaining to-do items for the mode.

Perhaps you could share the other to-do item with us so that we know what
else doesn't work.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: 23.0.50; vc-diff needs CVS repository now

[Eric S. Raymond]

Nick Roberts <nickrob@snap.net.nz>:
>  > This is one of my two remaining to-do items for the mode.
> 
> Perhaps you could share the other to-do item with us so that we know what
> else doesn't work.

The only other one is a minor item concerning what X-v = does when called
on a buffer visiting a file not under version control, but in a directory
which does have one or more responsible backends.  RMS and Stefan have 
a theory about how they want this to work, but I am still not certain
the theory is sound.  I need to take a hard look at this.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: 23.0.50; vc-diff needs CVS repository now

[Richard Stallman]

    The only other one is a minor item concerning what X-v = does when called
    on a buffer visiting a file not under version control, but in a directory
    which does have one or more responsible backends.

What does it do now in that case?

Re: 23.0.50; vc-diff needs CVS repository now

[Dan Nicolaescu]

"Eric S. Raymond" <esr@thyrsus.com> writes:

  > Glenn Morris <rgm@gnu.org>:
  > > Any comment on this? The part of vc-diff-internal that does a local
  > > diff where possible seems to have disappeared in the 2007-10-10 change
  > > "rewrite for filesets".
  > 
  > That's right, because the front end was the wrong place to do it.  
  > Modern VCS diff tools automatically stay local when they can.
  > Subversion diff does the right thing, and it's not even an issue
  > in the 3G systems (they keep local repositories).  It's the
  > kind of variation that should be handled in the VCS-specific back ends.
  > 
  > Actually CVS is the only awkward case.  So what ought to happen is
  > for something like the old stay-local logic to be implemented in the CVS 
  > back end.
  > 
  > This is one of my two remaining to-do items for the mode.  But someone
  > who is more of a CVS expert than I might get it done faster.

I adapted the old vc-diff-internal logic to vc-cvs-diff. After the
patch below the CVS repository is not needed if backup files are
present.

Not sure what is the "correct" return value for vc-cvs-diff in the
case when the CVS repository is not accessed (i.e. when using plain
"diff" instead of "cvs diff").

*** vc-cvs.el.~1.93.~	Fri Nov  9 22:02:03 2007
--- vc-cvs.el	Sun Nov 11 12:38:37 2007
***************
*** 515,526 ****
    "Get a difference report using CVS between two revisions of FILE."
      (let* ((async (and (not vc-disable-async-diff) 
  		       (vc-stay-local-p files)))
! 	   (status (apply 'vc-cvs-command (or buffer "*vc-diff*")
! 			  (if async 'async 1)
! 			  files "diff"
! 			  (and oldvers (concat "-r" oldvers))
! 			  (and newvers (concat "-r" newvers))
! 			  (vc-switches 'CVS 'diff))))
        (if async 1 status)))		; async diff, pessimistic assumption
  
  (defun vc-cvs-diff-tree (dir &optional rev1 rev2)
--- 515,554 ----
    "Get a difference report using CVS between two revisions of FILE."
      (let* ((async (and (not vc-disable-async-diff) 
  		       (vc-stay-local-p files)))
! 	   (invoke-cvs-diff-list nil)
! 	   status)
!       ;; Look through the file list and see if any files have backups
!       ;; that can be used to do a plain "diff" instead of "cvs diff".
!       (dolist (file files)
! 	(let ((ov oldvers)
! 	      (nv newvers))
! 	  (when (or (not ov) (string-equal ov ""))
! 	    (setq ov (vc-workfile-version file)))
! 	  (when (string-equal nv "")
! 	    (setq nv nil))
! 	  (let ((file-oldvers (vc-version-backup-file file ov))
! 		(file-newvers (if (not nv)
! 				  file
! 				(vc-version-backup-file file nv)))
! 		(coding-system-for-read (vc-coding-system-for-diff file)))
! 	    (if (and file-oldvers file-newvers)
! 		(apply 'vc-do-command (or buffer "*vc-diff*") 1 "diff" nil
! 		       (append (if (listp diff-switches)
! 				   diff-switches
! 				 (list diff-switches))
! 			       (if (listp vc-diff-switches)
! 				   vc-diff-switches
! 				 (list vc-diff-switches))
! 			       (list (file-relative-name file-oldvers)
! 				     (file-relative-name file-newvers))))
! 	      (push file invoke-cvs-diff-list)))))
!       (when invoke-cvs-diff-list
! 	(setq status (apply 'vc-cvs-command (or buffer "*vc-diff*")
! 			    (if async 'async 1)
! 			    invoke-cvs-diff-list "diff"
! 			    (and oldvers (concat "-r" oldvers))
! 			    (and newvers (concat "-r" newvers))
! 			    (vc-switches 'CVS 'diff))))
        (if async 1 status)))		; async diff, pessimistic assumption
  
  (defun vc-cvs-diff-tree (dir &optional rev1 rev2)

Re: 23.0.50; vc-diff needs CVS repository now

[Eric S. Raymond]

Dan Nicolaescu <dann@ics.uci.edu>:
> I adapted the old vc-diff-internal logic to vc-cvs-diff. After the
> patch below the CVS repository is not needed if backup files are
> present.

Thanks, Dan.  If you have not yet committed this, you have my encouragement
to do so.
 
> Not sure what is the "correct" return value for vc-cvs-diff in the
> case when the CVS repository is not accessed (i.e. when using plain
> "diff" instead of "cvs diff").

Hmmm...probably 0.  But even if that's wrong, your logic is still an
improvement.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: 23.0.50; vc-diff needs CVS repository now

[Dan Nicolaescu]

"Eric S. Raymond" <esr@thyrsus.com> writes:

  > Dan Nicolaescu <dann@ics.uci.edu>:
  > > I adapted the old vc-diff-internal logic to vc-cvs-diff. After the
  > > patch below the CVS repository is not needed if backup files are
  > > present.
  > 
  > Thanks, Dan.  If you have not yet committed this, you have my encouragement
  > to do so.

Thanks. Done.

Re: 23.0.50; vc-diff needs CVS repository now

[Nick Roberts]

 > I adapted the old vc-diff-internal logic to vc-cvs-diff. After the
 > patch below the CVS repository is not needed if backup files are
 > present.

This change looks good to me, so I've removed the corresponding item in
FOR-RELEASE.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: 23.0.50; vc-diff needs CVS repository now

[Stefan Monnier]

> I adapted the old vc-diff-internal logic to vc-cvs-diff.  After the
> patch below the CVS repository is not needed if backup files are
> present.

Thanks.  It might be a good occasion to fix a bug in VC: backup files
should only be used this way if they refer to a precise revision number,
but not if they refer to a tag or (worse) a branch, since tags and
branches can change.


        Stefan