C-x v v no longer works the way it used to

C-x v v no longer works the way it used to

[Jim Meyering]

Before the recent vc changes, in a version-controlled directory,
if I were visiting a non-version-controlled file and hit C-x v v,
it would effectively add that single file to the version control
system.  i.e., cvs add, git add, etc.

Now, all it does is print "No fileset is available here."

Is this intentional?
If so, how should one add a single file without resorting
to first marking it in dired mode.

To demonstrate, using git, I did this:

  mkdir k && cd k && git-init && touch a && git-add a && git-commit -m . &&
    echo x > new && emacs new

Then I typed C-x v v.

Same story using CVS.

Re: C-x v v no longer works the way it used to

[Eric Hanchrow]

>>>>> "Jim" == Jim Meyering <jim@meyering.net> writes:

    Jim> Before the recent vc changes, in a version-controlled
    Jim> directory, if I were visiting a non-version-controlled file
    Jim> and hit C-x v v, it would effectively add that single file to
    Jim> the version control system.  i.e., cvs add, git add, etc.

    Jim> Now, all it does is print "No fileset is available here."

    Jim> Is this intentional?  

I assume so.
                               
    Jim> If so, how should one add a single file without resorting to
    Jim> first marking it in dired mode.

C-x v i

-- 
But users will not now with glad cries glom on to a language that
gives them no more than what Scheme or Pascal gave them.
        -- Guy Steele, http://www.sun.com/research/jtech/pubs/98-oopsla-growing.ps

Re: C-x v v no longer works the way it used to

[Miles Bader]

Eric Hanchrow <offby1@blarg.net> writes:
>     Jim> if I were visiting a non-version-controlled file and hit C-x
>     Jim> v v, it would effectively add that single file to the version
>     Jim> control system.  i.e., cvs add, git add, etc.
>
>     Jim> Now, all it does is print "No fileset is available here."
>     Jim> Is this intentional?  
>
> I assume so.

If so, it's arguably wrong.  "C-x v v" is "do the right thing for
version control", not "commit this file".

Even if new behavior _is_ desirable, the error message is very vague and
confusing; something like "The file NAME is not under revision control;
use <binding> to add it" would be much more useful.

I think maybe some good guidelines might be:

  1) Don't use unusual terms like "fileset" in error messages unless
     it's impossible to do better.

  2) Try to refer to something concrete that the user can associate with
     the command he gave (one could use a helper function for signalling
     vc errors that would use alternative phrasing depending on the
     situation, e.g., "file FOO" for a single file, and "files FOO, BAR,
     BAZ" for multiple files, etc).

  3) Don't use vague phrasing like "not available here".  What does
     "here" refer to -- the machine, the current directory, some
     internal code state...?  What does "not available" mean?  It sounds
     like it could be a temporary error...

-Miles

-- 
((lambda (x) (list x x)) (lambda (x) (list x x)))

Re: C-x v v no longer works the way it used to

[Stefan Monnier]

> Before the recent vc changes, in a version-controlled directory,
> if I were visiting a non-version-controlled file and hit C-x v v,
> it would effectively add that single file to the version control
> system.  i.e., cvs add, git add, etc.

> Now, all it does is print "No fileset is available here."

> Is this intentional?

It's a bug.


        Stefan

Re: C-x v v no longer works the way it used to

[Eric S. Raymond]

Stefan Monnier <monnier@iro.umontreal.ca>:
> > Before the recent vc changes, in a version-controlled directory,
> > if I were visiting a non-version-controlled file and hit C-x v v,
> > it would effectively add that single file to the version control
> > system.  i.e., cvs add, git add, etc.
> 
> > Now, all it does is print "No fileset is available here."
> 
> > Is this intentional?
> 
> It's a bug.

Correct.  It's against my original intention for how the mode was supposed 
to work.  Somebody broke that feature years ago; I didn't restore it during
the rewrite because I was being ultraconservative about behavior changes.

I'll fix it as soon as I get sufficiently out from under this bout of
gastroenteritis...
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: C-x v v no longer works the way it used to

[Dan Nicolaescu]

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

  > > Before the recent vc changes, in a version-controlled directory,
  > > if I were visiting a non-version-controlled file and hit C-x v v,
  > > it would effectively add that single file to the version control
  > > system.  i.e., cvs add, git add, etc.
  > 
  > > Now, all it does is print "No fileset is available here."
  > 
  > > Is this intentional?
  > 
  > It's a bug.

This patch fixes it. 
vc-next-action still has the logic to deal with unregistered files,
but vc-deduce-fileset would not return one... 
This patch is technically incorrect because it changes
vc-deduce-fileset to not do what it's docs says it has to do.
Not sure how the new VC design is supposed to work to fix it
properly... 

Index: vc.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc.el,v
retrieving revision 1.473
diff -c -3 -p -c -r1.473 vc.el
*** vc.el  19 Oct 2007 20:59:49 -0000   1.473
--- vc.el  19 Oct 2007 22:47:56 -0000
*************** Otherwise, throw an error."
*** 1283,1288 ****
--- 1282,1288 ----
       (message "All version-controlled files below %s selected."
       		 default-directory)
       (list default-directory)))
+   ((not (vc-registered buffer-file-name)) (list buffer-file-name))
    (t (error "No fileset is available here."))))

Re: C-x v v no longer works the way it used to

[Jim Meyering]

Dan Nicolaescu <dann@ics.uci.edu> wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>   > > Before the recent vc changes, in a version-controlled directory,
>   > > if I were visiting a non-version-controlled file and hit C-x v v,
>   > > it would effectively add that single file to the version control
>   > > system.  i.e., cvs add, git add, etc.
>   >
>   > > Now, all it does is print "No fileset is available here."
>   >
>   > > Is this intentional?
>   >
>   > It's a bug.
>
> This patch fixes it.
> vc-next-action still has the logic to deal with unregistered files,
> but vc-deduce-fileset would not return one...
> This patch is technically incorrect because it changes
> vc-deduce-fileset to not do what it's docs says it has to do.
> Not sure how the new VC design is supposed to work to fix it
> properly...
>
> Index: vc.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/vc.el,v
> retrieving revision 1.473
> diff -c -3 -p -c -r1.473 vc.el
> *** vc.el  19 Oct 2007 20:59:49 -0000   1.473
> --- vc.el  19 Oct 2007 22:47:56 -0000
> *************** Otherwise, throw an error."
> *** 1283,1288 ****
> --- 1282,1288 ----
>        (message "All version-controlled files below %s selected."
>        		 default-directory)
>        (list default-directory)))
> +   ((not (vc-registered buffer-file-name)) (list buffer-file-name))
>     (t (error "No fileset is available here."))))

Thanks!
Works for me, though I'd rather it not prompt with
"Initial revision level for FILE_NAME: "

BTW, your patch didn't apply, so I applied it manually.

Re: C-x v v no longer works the way it used to

[Eric S. Raymond]

Dan Nicolaescu <dann@ics.uci.edu>:
> This patch fixes it. 
> vc-next-action still has the logic to deal with unregistered files,
> but vc-deduce-fileset would not return one... 
> This patch is technically incorrect because it changes
> vc-deduce-fileset to not do what it's docs says it has to do.
> Not sure how the new VC design is supposed to work to fix it
> properly... 

You had the right general idea; the strategy has to go through 
vc-deduce-fileset in order not to break the design.  But your patch
produces behavior that won't be welcome in commands other than
vc-next-action, so I put the arm you added under control of 
a new boolean argument ALLOW-UNREGISTERED.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: C-x v v no longer works the way it used to

[Dan Nicolaescu]

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

  > Dan Nicolaescu <dann@ics.uci.edu>:
  > > This patch fixes it. 
  > > vc-next-action still has the logic to deal with unregistered files,
  > > but vc-deduce-fileset would not return one... 
  > > This patch is technically incorrect because it changes
  > > vc-deduce-fileset to not do what it's docs says it has to do.
  > > Not sure how the new VC design is supposed to work to fix it
  > > properly... 
  > 
  > You had the right general idea; the strategy has to go through 
  > vc-deduce-fileset in order not to break the design.  But your patch
  > produces behavior that won't be welcome in commands other than
  > vc-next-action, so I put the arm you added under control of 
  > a new boolean argument ALLOW-UNREGISTERED.

Now this points to another issue: 
vc-next-action does this:

      (mapc 'vc-register files))

But vc-register is defined like this:

(defun vc-register (&optional set-revision comment)

i.e. it does not have a file parameter, it works on the current
buffer.

The easy fix would probably be to change the mapc call to do a
with-current-buffer. But it might be better to change vc-register to
take a fileset parameter... 

Re: C-x v v no longer works the way it used to

[Dan Nicolaescu]

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

  > "Eric S. Raymond" <esr@thyrsus.com> writes:
  > 
  >   > Dan Nicolaescu <dann@ics.uci.edu>:
  >   > > This patch fixes it. 
  >   > > vc-next-action still has the logic to deal with unregistered files,
  >   > > but vc-deduce-fileset would not return one... 
  >   > > This patch is technically incorrect because it changes
  >   > > vc-deduce-fileset to not do what it's docs says it has to do.
  >   > > Not sure how the new VC design is supposed to work to fix it
  >   > > properly... 
  >   > 
  >   > You had the right general idea; the strategy has to go through 
  >   > vc-deduce-fileset in order not to break the design.  But your patch
  >   > produces behavior that won't be welcome in commands other than
  >   > vc-next-action, so I put the arm you added under control of 
  >   > a new boolean argument ALLOW-UNREGISTERED.
  > 
  > Now this points to another issue: 
  > vc-next-action does this:
  > 
  >       (mapc 'vc-register files))
  > 
  > But vc-register is defined like this:
  > 
  > (defun vc-register (&optional set-revision comment)
  > 
  > i.e. it does not have a file parameter, it works on the current
  > buffer.
  > 
  > The easy fix would probably be to change the mapc call to do a
  > with-current-buffer. But it might be better to change vc-register to
  > take a fileset parameter... 


Here's a patch that changes vc-register to take a file parameter. In the
end should take a fileset, but it seems better to first make sure that
the simpler case works.

Jim: this also should not ask the question about "Initial revision
level".

Should I check this in?

Index: vc.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc.el,v
retrieving revision 1.480
diff -c -3 -p -c -r1.480 vc.el
*** vc.el	11 Nov 2007 14:55:55 -0000	1.480
--- vc.el	12 Nov 2007 15:19:51 -0000
*************** merge in the changes into your working c
*** 1535,1542 ****
    (vc-call-backend backend 'create-repo))
  
  ;;;###autoload
! (defun vc-register (&optional set-revision comment)
!   "Register the current file into a version control system.
  With prefix argument SET-REVISION, allow user to specify initial revision
  level.  If COMMENT is present, use that as an initial comment.
  
--- 1535,1543 ----
    (vc-call-backend backend 'create-repo))
  
  ;;;###autoload
! (defun vc-register (&optional fname set-revision comment)
!   "Register into a version control system.
! If FNAME is given register that file, otherwise register the current file.
  With prefix argument SET-REVISION, allow user to specify initial revision
  level.  If COMMENT is present, use that as an initial comment.
  
*************** directory are already registered under t
*** 1547,1586 ****
  register the file.  If no backend declares itself responsible, the
  first backend that could register the file is used."
    (interactive "P")
!   (unless buffer-file-name (error "No visited file"))
!   (when (vc-backend buffer-file-name)
!     (if (vc-registered buffer-file-name)
! 	(error "This file is already registered")
!       (unless (y-or-n-p "Previous master file has vanished.  Make a new one? ")
! 	(error "Aborted"))))
!   ;; Watch out for new buffers of size 0: the corresponding file
!   ;; does not exist yet, even though buffer-modified-p is nil.
!   (if (and (not (buffer-modified-p))
! 	   (zerop (buffer-size))
! 	   (not (file-exists-p buffer-file-name)))
!       (set-buffer-modified-p t))
!   (vc-buffer-sync)
  
!   (vc-start-entry (list buffer-file-name)
!                   (if set-revision
!                       (read-string (format "Initial revision level for %s: "
! 					   (buffer-name)))
! 		    (vc-call-backend (vc-responsible-backend buffer-file-name)
! 				     'init-revision))
!                   (or comment (not vc-initial-comment))
! 		  nil
!                   "Enter initial comment."
! 		  (lambda (files rev comment)
! 		    (dolist (file files)
! 		      (message "Registering %s... " file)
! 		      (let ((backend (vc-responsible-backend file t)))
! 			(vc-file-clearprops file)
! 			(vc-call-backend backend 'register (list file) rev comment)
! 			(vc-file-setprop file 'vc-backend backend)
! 			(unless vc-make-backup-files
! 			  (make-local-variable 'backup-inhibited)
! 			  (setq backup-inhibited t)))
! 		      (message "Registering %s... done" file)))))
  
  (defun vc-register-with (backend)
    "Register the current file with a specified back end."
--- 1548,1591 ----
  register the file.  If no backend declares itself responsible, the
  first backend that could register the file is used."
    (interactive "P")
!   (when (and (null fname) (null buffer-file-name)) (error "No visited file"))
  
!   (let ((bname (if fname (get-file-buffer fname) buffer-file-name)))
!     (unless fname (setq fname buffer-file-name))
!     (when bname
!       (with-current-buffer bname
! 	(when (vc-backend buffer-file-name)
! 	  (if (vc-registered buffer-file-name)
! 	      (error "This file is already registered")
! 	    (unless (y-or-n-p "Previous master file has vanished.  Make a new one? ")
! 	      (error "Aborted"))))
! 	;; Watch out for new buffers of size 0: the corresponding file
! 	;; does not exist yet, even though buffer-modified-p is nil.
! 	(if (and (not (buffer-modified-p))
! 		 (zerop (buffer-size))
! 		 (not (file-exists-p buffer-file-name)))
! 	    (set-buffer-modified-p t))
! 	(vc-buffer-sync)))
!     (vc-start-entry (list fname)
! 		    (if set-revision
! 			(read-string (format "Initial revision level for %s: "
! 					     fname))
! 		      (vc-call-backend (vc-responsible-backend fname)
! 				       'init-revision))
! 		    (or comment (not vc-initial-comment))
! 		    nil
! 		    "Enter initial comment."
! 		    (lambda (files rev comment)
! 		      (dolist (file files)
! 			(message "Registering %s... " file)
! 			(let ((backend (vc-responsible-backend file t)))
! 			  (vc-file-clearprops file)
! 			  (vc-call-backend backend 'register (list file) rev comment)
! 			  (vc-file-setprop file 'vc-backend backend)
! 			  (unless vc-make-backup-files
! 			    (make-local-variable 'backup-inhibited)
! 			    (setq backup-inhibited t)))
! 			(message "Registering %s... done" file))))))
  
  (defun vc-register-with (backend)
    "Register the current file with a specified back end."


Diffs between working revision and workfile end here.

Re: C-x v v no longer works the way it used to

[Dan Nicolaescu]

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

  > Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  >   > "Eric S. Raymond" <esr@thyrsus.com> writes:
  >   > 
  >   >   > Dan Nicolaescu <dann@ics.uci.edu>:
  >   >   > > This patch fixes it. 
  >   >   > > vc-next-action still has the logic to deal with unregistered files,
  >   >   > > but vc-deduce-fileset would not return one... 
  >   >   > > This patch is technically incorrect because it changes
  >   >   > > vc-deduce-fileset to not do what it's docs says it has to do.
  >   >   > > Not sure how the new VC design is supposed to work to fix it
  >   >   > > properly... 
  >   >   > 
  >   >   > You had the right general idea; the strategy has to go through 
  >   >   > vc-deduce-fileset in order not to break the design.  But your patch
  >   >   > produces behavior that won't be welcome in commands other than
  >   >   > vc-next-action, so I put the arm you added under control of 
  >   >   > a new boolean argument ALLOW-UNREGISTERED.
  >   > 
  >   > Now this points to another issue: 
  >   > vc-next-action does this:
  >   > 
  >   >       (mapc 'vc-register files))
  >   > 
  >   > But vc-register is defined like this:
  >   > 
  >   > (defun vc-register (&optional set-revision comment)
  >   > 
  >   > i.e. it does not have a file parameter, it works on the current
  >   > buffer.
  >   > 
  >   > The easy fix would probably be to change the mapc call to do a
  >   > with-current-buffer. But it might be better to change vc-register to
  >   > take a fileset parameter... 
  > 
  > 
  > Here's a patch that changes vc-register to take a file parameter. In the
  > end should take a fileset, but it seems better to first make sure that
  > the simpler case works.

I checked this in.

Making vc-register have a fileset parameter would probably have to wait
until there's a way to test that. Currently vc-dired does not show
non-registered files, so one can't select them to create a fileset.

Re: C-x v v no longer works the way it used to

[Jim Meyering]

Dan Nicolaescu <dann@ics.uci.edu> wrote:
> I checked this in.

Thanks.
Works for me.