vc-register complains if a file is already registered in a git repository

vc-register complains if a file is already registered in a git repository

[Phil Hagelberg]

When I visit a file that is already registered in my git repository, I
would like to make some changes and register them in the git index.

Unfortunately when I use vc-register on a file that is already in the
repository, it complains, saying "This file is already registered". This
is a reasonable thing to do in most VC systems, but git behaves
differently since it's a common thing to want to register the changes
you've just made to the git index (or staging area).

I would suggest that the vc-register be allowed when the file is
already registered if the backend is git. I know that VC intends to be a
fairly transparent frontend that behaves the same way with all different
version control systems, but ignoring the existence of the git index
discards a lot of the flexibility that is the reason many people use git
in the first place.

Maybe if a special-case within vc.el itself is frowned upon a new
command such as vc-register-changes could be made that is only
implemented in the git backend?

-Phil

Re: vc-register complains if a file is already registered in a git repository

[Stefan Monnier]

> Unfortunately when I use vc-register on a file that is already in the
> repository, it complains, saying "This file is already registered". This
> is a reasonable thing to do in most VC systems, but git behaves
> differently since it's a common thing to want to register the changes
> you've just made to the git index (or staging area).

I'm not sure we want to allow C-x v i to do that.  OT1H it might be
convenient and maybe it can work well, but OTOH it looks risky and would
require changing vc-register more than I'd like.

It would probably be better for vc-git to provide another command, for that.
We could probably add a `extra-keys' operation similar to `extra-menu'
so that vc-git can not only provide a new command but also provide
a specific C-x v <foo> key binding for it.


        Stefan

Re: vc-register complains if a file is already registered in a git repository

[Phil Hagelberg]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Unfortunately when I use vc-register on a file that is already in the
>> repository, it complains, saying "This file is already registered". This
>> is a reasonable thing to do in most VC systems, but git behaves
>> differently since it's a common thing to want to register the changes
>> you've just made to the git index (or staging area).
>
> I'm not sure we want to allow C-x v i to do that.  OT1H it might be
> convenient and maybe it can work well, but OTOH it looks risky and would
> require changing vc-register more than I'd like.

I just noticed the existence of the vc-default-could-register function:

  Return non-nil if BACKEND could be used to register FILE.

It seems like vc-register should be running this function instead of
vc-registered to check if it should continue, like this:

diff --git a/lisp/vc.el b/lisp/vc.el
index 35e846d..75189e5 100644
--- a/lisp/vc.el
+++ b/lisp/vc.el
@@ -1206,11 +1206,10 @@ first backend that could register the file is used."
     (dolist (fname files)
       (let ((bname (get-file-buffer fname)))
 	(unless fname (setq fname buffer-file-name))
-	(when (vc-backend fname)
-	  (if (vc-registered fname)
-	      (error "This file is already registered")
-	    (unless (y-or-n-p "Previous master file has vanished.  Make a new one? ")
-	      (error "Aborted"))))
+	(let ((backend (vc-backend fname)))
+	  (if (and backend
+                   (not (vc-call-backend backend 'could-register fname)))
+	      (error "This file cannot be registered")))
 	;; Watch out for new buffers of size 0: the corresponding file
 	;; does not exist yet, even though buffer-modified-p is nil.

But in most backends, vc-BACKEND-could-register only checks to make sure
the containing directory is being tracked rather than checking to make
sure the file isn't already registered, so you're right that it would
require significant changes.

Since we're in a feature freeze, perhaps this is ill-advised?

> It would probably be better for vc-git to provide another command, for that.
> We could probably add a `extra-keys' operation similar to `extra-menu'
> so that vc-git can not only provide a new command but also provide
> a specific C-x v <foo> key binding for it.

This would be great if we don't want to make the changes above. I can
add it. Could I get some details about extra-menu though? I'm not
familiar with it.

-Phil
http://technomancy.us

Re: vc-register complains if a file is already registered in a git repository

[Stefan Monnier]

> I just noticed the existence of the vc-default-could-register function:

>   Return non-nil if BACKEND could be used to register FILE.

> It seems like vc-register should be running this function instead of
> vc-registered to check if it should continue, like this:

No, could-register is used for other purposes.

> Since we're in a feature freeze, perhaps this is ill-advised?

Yes.

>> It would probably be better for vc-git to provide another command, for that.
>> We could probably add a `extra-keys' operation similar to `extra-menu'
>> so that vc-git can not only provide a new command but also provide
>> a specific C-x v <foo> key binding for it.

> This would be great if we don't want to make the changes above. I can
> add it. Could I get some details about extra-menu though? I'm not
> familiar with it.

Grep for extra-menu and extra-status-menu (which should be renamed
extra-dir-menu, BTW) in vc*.el.


        Stefan

Re: vc-register complains if a file is already registered in a git repository

[Phil Hagelberg]

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

>>> It would probably be better for vc-git to provide another command, for that.
>>> We could probably add a `extra-keys' operation similar to `extra-menu'
>>> so that vc-git can not only provide a new command but also provide
>>> a specific C-x v <foo> key binding for it.
>
>> This would be great if we don't want to make the changes above. I can
>> add it. Could I get some details about extra-menu though? I'm not
>> familiar with it.
>
> Grep for extra-menu and extra-status-menu (which should be renamed
> extra-dir-menu, BTW) in vc*.el.

I've added the vc-git-register-changes command to vc-git-extra-menu-map
and will start work on creating extra-keys functionality that works like
extra-menu.

-Phil

diff --git a/lisp/vc-git.el b/lisp/vc-git.el
index 51ccc5d..773e6d3 100644
--- a/lisp/vc-git.el
+++ b/lisp/vc-git.el
@@ -622,6 +622,9 @@ or BRANCH^ (where \"^\" can be repeated)."
     (define-key map [git-grep]
       '(menu-item "Git grep..." vc-git-grep
 		  :help "Run the `git grep' command"))
+    (define-key map [git-register-changes]
+      '(menu-item "Register changes" vc-git-register-changes
+                  :help "Register the changes in the current buffer"))
     map))
 
 (defun vc-git-extra-menu () vc-git-extra-menu-map)
@@ -681,6 +684,12 @@ This command shares argument histories with \\[rgrep] and \\[grep]."
 	  (compilation-start command 'grep-mode))
 	(if (eq next-error-last-buffer (current-buffer))
 	    (setq default-directory dir))))))
+
+(defun vc-git-register-changes ()
+  "Register the changes in the current file to the staging area."
+  (interactive)
+  (vc-git-register buffer-file-name))
+
 \f
 ;;; Internal commands
 

Re: vc-register complains if a file is already registered in a git repository

[Phil Hagelberg]

Phil Hagelberg <phil@hagelb.org> writes:

> I've added the vc-git-register-changes command to vc-git-extra-menu-map
> and will start work on creating extra-keys functionality that works like
> extra-menu.

OK, here's the cumulative patch for both features.

I am not sure I completely understand the use of fset for
vc-prefix-map. I tried converting it to a regular function definition,
but that did not work; it made the prefix command essentially a
no-op. So someone who's familiar with prefix keymaps should review this
patch before it's applied.

-Phil

diff --git a/lisp/vc-git.el b/lisp/vc-git.el
index 51ccc5d..d3ac624 100644
--- a/lisp/vc-git.el
+++ b/lisp/vc-git.el
@@ -622,6 +622,9 @@ or BRANCH^ (where \"^\" can be repeated)."
     (define-key map [git-grep]
       '(menu-item "Git grep..." vc-git-grep
 		  :help "Run the `git grep' command"))
+    (define-key map [git-register-changes]
+      '(menu-item "Register changes" vc-git-register-changes
+                  :help "Register the changes in the current buffer"))
     map))
 
 (defun vc-git-extra-menu () vc-git-extra-menu-map)
@@ -681,6 +684,18 @@ This command shares argument histories with \\[rgrep] and \\[grep]."
 	  (compilation-start command 'grep-mode))
 	(if (eq next-error-last-buffer (current-buffer))
 	    (setq default-directory dir))))))
+
+(defun vc-git-register-changes ()
+  "Register the changes in the current file to the staging area."
+  (interactive)
+  (vc-git-register buffer-file-name)
+  (message "Registered changes to %s... " buffer-file-name))
+
+(defun vc-git-append-extra-keys (original-map)
+  (let ((extra-keys-map (copy-keymap original-map)))
+    (define-key extra-keys-map "e" 'vc-git-register-changes)
+    extra-keys-map))
+
 \f
 ;;; Internal commands
 
diff --git a/lisp/vc-hooks.el b/lisp/vc-hooks.el
index 97dca35..53e0e31 100644
--- a/lisp/vc-hooks.el
+++ b/lisp/vc-hooks.el
@@ -970,7 +970,15 @@ Used in `find-file-not-found-functions'."
     (define-key map "=" 'vc-diff)
     (define-key map "~" 'vc-revision-other-window)
     map))
-(fset 'vc-prefix-map vc-prefix-map)
+
+(fset 'vc-prefix-map
+      (when vc-mode
+        (vc-call-backend
+         (if buffer-file-name
+             (vc-backend buffer-file-name)
+           (vc-responsible-backend default-directory))
+         'append-extra-keys vc-prefix-map)))
+
 (define-key global-map "\C-xv" 'vc-prefix-map)
 
 (defvar vc-menu-map

Re: vc-register complains if a file is already registered in a git repository

[Phil Hagelberg]

Phil Hagelberg <phil@hagelb.org> writes:

> Phil Hagelberg <phil@hagelb.org> writes:
>
>> I've added the vc-git-register-changes command to vc-git-extra-menu-map
>> and will start work on creating extra-keys functionality that works like
>> extra-menu.
>
> OK, here's the cumulative patch for both features.

Disregard that patch; it runs the append-extra-keys invocation at load
time, not when the vc-prefix-map is invoked.

It's really unclear due to the way prefix maps work how you can append
something to the map when it is invoked. For the menu maps it looks like
there are filter functions that are meant to do that, but it I can't
find any way to do this for prefix keymaps.

I suspect it would be much more straightforward if vc-register-changes
were simply a top-level vc command that most backends simply didn't
implement.

-Phil