bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

In the *vc-dir* buffer, if the cursor is on a source file entry, "v"
operates on the file under the cursor.

However, if "v" is used on a non-file line, the behavior is kind of
weird:  It pops up a *vc-log* buffer, and allows you to enter a log
message, but then typing C-c C-c (`log-edit-done') in the *vc-log*
buffer to finish the commit results in an error:

   vc-find-backend-function: Cannot open load file: vc-nil

What I _expected_ to happen is for it to commit every changed file in
the *vc-dir* buffer; this behavior is not only fairly natural (I think),
but also suggested by actions of vc prior to the error:

  (1) Using the C-c C-f command (`log-edit-show-files') *vc-log* shows
      the directory name as the "files" (in *log-edit-files* buffer)

  (2) The C-c C-c command in *vc-log*, prior to the above error,
      displays the message (where DIRNAME is the directory name):

         Checking in DIRNAME...

Thanks,

-Miles


In GNU Emacs 23.0.91.10 (x86_64-unknown-linux-gnu, GTK+ Version 2.15.5)
 of 2009-03-12 on catnip
Windowing system distributor `The X.Org Foundation', version 11.0.10599902
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: ja_JP.UTF-8
  value of $XMODIFIERS: @im=SCIM
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: VC dir

Minor modes in effect:
  diff-auto-refine-mode: t
  vc-parent-buffer: *vc-dir*
  rcirc-track-minor-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
SPC t h e SPC " * " SPC o p e r a t i o n <escape> 
h s <backspace> o p e r a t i o n ) M-b M-b M-b M-b 
M-b M-b M-h M-f M-d <escape> g C-n , SPC i n s t e 
a d SPC o f SPC a s SPC a SPC n u m b e r SPC o f SPC 
s t o p s SPC ( " * + <backspace> <backspace> + " ) 
SPC a s SPC w a s SPC p r e v i o u s l y SPC d o n 
e . C-p <escape> b <escape> b <escape> h <escape> d 
C-d <escape> g C-n C-n <return> <return> T h i SPC 
i s SPC <escape> b C-b s C-e a n SPC i n c o m p a 
t i b l e SPC c h a n g e , SPC b u t SPC I SPC f i 
n d SPC i t SPC m o r e SPC i n C-p C-SPC C-n C-n t 
u i t i v e SPC ( I SPC w a s SPC C-p C-SPC C-n C-n 
C-w C-n C-n C-x C-s C-s C-s C-g C-g C-g C-p C-e C-p 
C-M-b M-b M-b a SPC <escape> f <backspace> <backspace> 
e <backspace> <backspace> e SPC f a c t o r <escape> 
g C-n C-n C-n C-c C-c C-x p C-n C-n C-x o C-x n C-x 
h <escape> w C-x p C-n C-n v C-c C-c C-x n C-x n C-n 
v C-c C-c C-x s e d e C-a C-x k <return> y e s <return> 
<escape> x r e p o r t - e m SPC <return>

Recent messages:
Mark set [3 times]
Press C-c C-c when you are done editing.
Enter a change comment.  Type C-c C-c when done
Checking in /home/miles/src/snogray/snogray/...
xding
vc-find-backend-function: Cannot open load file: vc-nil
Mark set
Press C-c C-c when you are done editing.
Enter a change comment.  Type C-c C-c when done
Checking in /home/miles/src/snogray/snogray/image-cmdline.cc...done

-- 
`To alcohol!  The cause of, and solution to,
 all of life's problems' --Homer J. Simpson

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > In the *vc-dir* buffer, if the cursor is on a source file entry, "v"
  > operates on the file under the cursor.
  > 
  > However, if "v" is used on a non-file line, the behavior is kind of
  > weird:  It pops up a *vc-log* buffer, and allows you to enter a log
  > message, but then typing C-c C-c (`log-edit-done') in the *vc-log*
  > buffer to finish the commit results in an error:
  > 
  >    vc-find-backend-function: Cannot open load file: vc-nil
  > 
  > What I _expected_ to happen is for it to commit every changed file in
  > the *vc-dir* buffer; this behavior is not only fairly natural (I think),
  > but also suggested by actions of vc prior to the error:
  > 
  >   (1) Using the C-c C-f command (`log-edit-show-files') *vc-log* shows
  >       the directory name as the "files" (in *log-edit-files* buffer)
  > 
  >   (2) The C-c C-c command in *vc-log*, prior to the above error,
  >       displays the message (where DIRNAME is the directory name):
  > 
  >          Checking in DIRNAME...


Can you please provide a way to reproduce this starting from emacs -Q?
What is the output of:

M-: (vc-deduce-fileset) RET

before you press "v" ? 

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

On Fri, Mar 13, 2009 at 9:38 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
>  > However, if "v" is used on a non-file line, the behavior is kind of
>  > weird:  It pops up a *vc-log* buffer, and allows you to enter a log
>  > message, but then typing C-c C-c (`log-edit-done') in the *vc-log*
>  > buffer to finish the commit results in an error:
>
> Can you please provide a way to reproduce this starting from emacs -Q?

  (1) cd to a source directory that uses git for source management

  (2) start emacs:  emacs -Q  -nw

  (3) modify any source file in that dir, e.g., add a new line "Hello"
to a README or something

  (4) Start vc-dir:  M-x vc-dir RET RET

  (5) In the *vc-dir* buffer, you'll see the file you modified listed;
the cursor should be initially positioned on the first line of the
buffer, i.e., not on any source file.

  (6) Without moving the cursor, try to commit:  v

  (7) Enter a log message and submit:   Oink OINK RET

  (8) You should get the error I mentioned

> What is the output of:
> M-: (vc-deduce-fileset) RET
> before you press "v" ?

   M-: (vc-deduce-fileset) RET
     => (Git ("/home/miles/src/snogray/snogray/") nil nil nil)

The directory name there is the top-level source directory.

Thanks,

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

Oh, I obviously forgot the commit part of step (7); it should be:

  (7) Enter a log message and submit:   Oink OINK RET C-c C-c

Thanks,

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Fri, Mar 13, 2009 at 9:38 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > >  > However, if "v" is used on a non-file line, the behavior is kind of
  > >  > weird:  It pops up a *vc-log* buffer, and allows you to enter a log
  > >  > message, but then typing C-c C-c (`log-edit-done') in the *vc-log*
  > >  > buffer to finish the commit results in an error:
  > >
  > > Can you please provide a way to reproduce this starting from emacs -Q?
  > 
  >   (1) cd to a source directory that uses git for source management
  > 
  >   (2) start emacs:  emacs -Q  -nw
  > 
  >   (3) modify any source file in that dir, e.g., add a new line "Hello"
  > to a README or something
  > 
  >   (4) Start vc-dir:  M-x vc-dir RET RET
  > 
  >   (5) In the *vc-dir* buffer, you'll see the file you modified listed;
  > the cursor should be initially positioned on the first line of the
  > buffer, i.e., not on any source file.
  > 
  >   (6) Without moving the cursor, try to commit:  v
  > 
  >   (7) Enter a log message and submit:   Oink OINK RET
  > 
  >   (8) You should get the error I mentioned
  > 
  > > What is the output of:
  > > M-: (vc-deduce-fileset) RET
  > > before you press "v" ?
  > 
  >    M-: (vc-deduce-fileset) RET
  >      => (Git ("/home/miles/src/snogray/snogray/") nil nil nil)

Ah, git.  Then is probably the same as bug#1589.
vc-git.el has problems with working with subdirectories. 

Do you get this error when you run vc-dir at the top of the git tree?

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

On Fri, Mar 13, 2009 at 10:34 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
> Ah, git.  Then is probably the same as bug#1589.
> vc-git.el has problems with working with subdirectories.
>
> Do you get this error when you run vc-dir at the top of the git tree?

My source directory only has one level, so... yes.

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

On Fri, Mar 13, 2009 at 10:50 AM, Miles Bader <miles@gnu.org> wrote:
> On Fri, Mar 13, 2009 at 10:34 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
>> Ah, git.  Then is probably the same as bug#1589.
>> vc-git.el has problems with working with subdirectories.
>>
>> Do you get this error when you run vc-dir at the top of the git tree?
>
> My source directory only has one level, so... yes.

BTW, Iooked at the entry for bug #1589, and I can't see any obvious
connection with my problem...

-Mile

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

Here's a script to create an appropriate git repo for testing:

#!/bin/sh
cd /tmp
rm -rf zonk
mkdir zonk
cd zonk
git init
echo plugh > ppling
git add .
git commit -m'Init' -a
echo Fnord >> ppling

Put that in a file, say "/tmp/mkbugrepo" and execute it with the shell
to make the repo, e.g.: sh /tmp/mkbugrepo

Now you can start emacs like:

   (cd /tmp/zonk; HOME=/tmp \emacs -nw -Q )

and continue with the rest of the bug recipe:

  M-x vc-dir RET RET
  v
  Blargh RET C-c C-c

Thanks,

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > Here's a script to create an appropriate git repo for testing:
  > 
  > #!/bin/sh
  > cd /tmp
  > rm -rf zonk
  > mkdir zonk
  > cd zonk
  > git init
  > echo plugh > ppling
  > git add .
  > git commit -m'Init' -a
  > echo Fnord >> ppling
  > 
  > Put that in a file, say "/tmp/mkbugrepo" and execute it with the shell
  > to make the repo, e.g.: sh /tmp/mkbugrepo
  > 
  > Now you can start emacs like:
  > 
  >    (cd /tmp/zonk; HOME=/tmp \emacs -nw -Q )
  > 
  > and continue with the rest of the bug recipe:
  > 
  >   M-x vc-dir RET RET
  >   v
  >   Blargh RET C-c C-c
  > 

Thanks.  
The immediate cause of the problem is that 
(vc-git-registered "/tmp/zonk/")
returns nil.

vc-git-registered is called because vc-finish-log-entry does a
`vc-call'.  `vc-call' tries to find the corresponding VC backend. 

Now, the VC backend should be know known at point when the log entry is
started, so there should be no need for the vc-call, vc-call-backend
could be used...

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

On Fri, Mar 13, 2009 at 4:27 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
> Thanks.
> The immediate cause of the problem is that
> (vc-git-registered "/tmp/zonk/")
> returns nil.
>
> vc-git-registered is called because vc-finish-log-entry does a
> `vc-call'.  `vc-call' tries to find the corresponding VC backend.
>
> Now, the VC backend should be know known at point when the log entry is
> started, so there should be no need for the vc-call, vc-call-backend
> could be used...

Is there anybody driving the design of the vc rewrite at this point?

Thanks,

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Fri, Mar 13, 2009 at 4:27 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > > Thanks.
  > > The immediate cause of the problem is that
  > > (vc-git-registered "/tmp/zonk/")
  > > returns nil.
  > >
  > > vc-git-registered is called because vc-finish-log-entry does a
  > > `vc-call'.  `vc-call' tries to find the corresponding VC backend.
  > >
  > > Now, the VC backend should be know known at point when the log entry is
  > > started, so there should be no need for the vc-call, vc-call-backend
  > > could be used...
  > 
  > Is there anybody driving the design of the vc rewrite at this point?

Not AFAIK.

But in this particular case it's not a question of redesign.
IMHO the best bet at this point is to get vc-git-registered fixed.
And maybe ask Stefan, maybe he knows the `vc-checkin' code better to say
if getting rid of `vc-call' is TRTD.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Fri, Mar 13, 2009 at 4:27 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > > Thanks.
  > > The immediate cause of the problem is that
  > > (vc-git-registered "/tmp/zonk/")
  > > returns nil.
  > >
  > > vc-git-registered is called because vc-finish-log-entry does a
  > > `vc-call'.  `vc-call' tries to find the corresponding VC backend.
  > >
  > > Now, the VC backend should be know known at point when the log entry is
  > > started, so there should be no need for the vc-call, vc-call-backend
  > > could be used...

You can try this patch that passes down the VC backend.  
Not tested beyond M-x byte-compile-file.

--- vc.el.~1.711.~	Thu Jan  8 14:24:05 2009
+++ vc.el	Fri Mar 13 09:14:58 2009
@@ -1085,13 +1085,13 @@
 	(if (not ready-for-commit)
 	    (message "No files remain to be committed")
 	  (if (not verbose)
-	      (vc-checkin ready-for-commit)
+	      (vc-checkin ready-for-commit backend)
 	    (progn
 	      (setq revision (read-string "New revision or backend: "))
 	      (let ((vsym (intern (upcase revision))))
 		(if (member vsym vc-handled-backends)
 		    (dolist (file files) (vc-transfer-file file vsym))
-		  (vc-checkin ready-for-commit revision))))))))
+		  (vc-checkin ready-for-commit backend revision))))))))
      ;; locked by somebody else (locking VCSes only)
      ((stringp state)
       ;; In the old days, we computed the revision once and used it on
@@ -1327,7 +1327,7 @@
      ".\n")
     (message "Please explain why you stole the lock.  Type C-c C-c when done.")))
 
-(defun vc-checkin (files &optional rev comment initial-contents)
+(defun vc-checkin (files backend &optional rev comment initial-contents)
   "Check in FILES.
 The optional argument REV may be a string specifying the new revision
 level (if nil increment the current level).  COMMENT is a comment
@@ -1341,6 +1341,7 @@
 Runs the normal hooks `vc-before-checkin-hook' and `vc-checkin-hook'."
   (when vc-before-checkin-hook
     (run-hooks 'vc-before-checkin-hook))
+  (lexical-let ((backend backend))
   (vc-start-logentry
    files rev comment initial-contents
    "Enter a change comment."
@@ -1356,13 +1357,13 @@
       ;; We used to change buffers to get local value of vc-checkin-switches,
       ;; but 'the' local buffer is not a well-defined concept for filesets.
       (progn
-	(vc-call checkin files rev comment)
+	(vc-call-backend backend 'checkin files rev comment)
 	(mapc 'vc-delete-automatic-version-backups files))
       `((vc-state . up-to-date)
 	(vc-checkout-time . ,(nth 5 (file-attributes file)))
 	(vc-working-revision . nil)))
      (message "Checking in %s...done" (vc-delistify files)))
-   'vc-checkin-hook))
+   'vc-checkin-hook)))
 
 ;;; Additional entry points for examining version histories
 
@@ -2096,7 +2097,7 @@
     (when (or move edited)
       (vc-file-setprop file 'vc-state 'edited)
       (vc-mode-line file)
-      (vc-checkin file nil comment (stringp comment)))))
+      (vc-checkin file new-backend nil comment (stringp comment)))))
 
 (defun vc-rename-master (oldmaster newfile templates)
   "Rename OLDMASTER to be the master file for NEWFILE based on TEMPLATES."

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

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

  > Miles Bader <miles@gnu.org> writes:
  > 
  >   > On Fri, Mar 13, 2009 at 4:27 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  >   > > Thanks.
  >   > > The immediate cause of the problem is that
  >   > > (vc-git-registered "/tmp/zonk/")
  >   > > returns nil.
  >   > >
  >   > > vc-git-registered is called because vc-finish-log-entry does a
  >   > > `vc-call'.  `vc-call' tries to find the corresponding VC backend.
  >   > >
  >   > > Now, the VC backend should be know known at point when the log entry is
  >   > > started, so there should be no need for the vc-call, vc-call-backend
  >   > > could be used...
  > 
  > You can try this patch that passes down the VC backend.  
  > Not tested beyond M-x byte-compile-file.

I have now tested this, and it seems to work fine.

But I don't really want to check it in at this point without an explicit
request from Stefan.


  > --- vc.el.~1.711.~	Thu Jan  8 14:24:05 2009
  > +++ vc.el	Fri Mar 13 09:14:58 2009
  > @@ -1085,13 +1085,13 @@
  >  	(if (not ready-for-commit)
  >  	    (message "No files remain to be committed")
  >  	  (if (not verbose)
  > -	      (vc-checkin ready-for-commit)
  > +	      (vc-checkin ready-for-commit backend)
  >  	    (progn
  >  	      (setq revision (read-string "New revision or backend: "))
  >  	      (let ((vsym (intern (upcase revision))))
  >  		(if (member vsym vc-handled-backends)
  >  		    (dolist (file files) (vc-transfer-file file vsym))
  > -		  (vc-checkin ready-for-commit revision))))))))
  > +		  (vc-checkin ready-for-commit backend revision))))))))
  >       ;; locked by somebody else (locking VCSes only)
  >       ((stringp state)
  >        ;; In the old days, we computed the revision once and used it on
  > @@ -1327,7 +1327,7 @@
  >       ".\n")
  >      (message "Please explain why you stole the lock.  Type C-c C-c when done.")))
  >  
  > -(defun vc-checkin (files &optional rev comment initial-contents)
  > +(defun vc-checkin (files backend &optional rev comment initial-contents)
  >    "Check in FILES.
  >  The optional argument REV may be a string specifying the new revision
  >  level (if nil increment the current level).  COMMENT is a comment
  > @@ -1341,6 +1341,7 @@
  >  Runs the normal hooks `vc-before-checkin-hook' and `vc-checkin-hook'."
  >    (when vc-before-checkin-hook
  >      (run-hooks 'vc-before-checkin-hook))
  > +  (lexical-let ((backend backend))
  >    (vc-start-logentry
  >     files rev comment initial-contents
  >     "Enter a change comment."
  > @@ -1356,13 +1357,13 @@
  >        ;; We used to change buffers to get local value of vc-checkin-switches,
  >        ;; but 'the' local buffer is not a well-defined concept for filesets.
  >        (progn
  > -	(vc-call checkin files rev comment)
  > +	(vc-call-backend backend 'checkin files rev comment)
  >  	(mapc 'vc-delete-automatic-version-backups files))
  >        `((vc-state . up-to-date)
  >  	(vc-checkout-time . ,(nth 5 (file-attributes file)))
  >  	(vc-working-revision . nil)))
  >       (message "Checking in %s...done" (vc-delistify files)))
  > -   'vc-checkin-hook))
  > +   'vc-checkin-hook)))
  >  
  >  ;;; Additional entry points for examining version histories
  >  
  > @@ -2096,7 +2097,7 @@
  >      (when (or move edited)
  >        (vc-file-setprop file 'vc-state 'edited)
  >        (vc-mode-line file)
  > -      (vc-checkin file nil comment (stringp comment)))))
  > +      (vc-checkin file new-backend nil comment (stringp comment)))))
  >  
  >  (defun vc-rename-master (oldmaster newfile templates)
  >    "Rename OLDMASTER to be the master file for NEWFILE based on TEMPLATES."

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Stefan Monnier]

> But I don't really want to check it in at this point without an explicit
> request from Stefan.

Removing calls to determine the backend (and replace them with backend
info propagated from elsewhere) is generally a good thing.  I think your
patch looks good.


        Stefan

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

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

  > > But I don't really want to check it in at this point without an explicit
  > > request from Stefan.
  > 
  > Removing calls to determine the backend (and replace them with backend
  > info propagated from elsewhere) is generally a good thing.  I think your
  > patch looks good.

Thanks.  I checked it in.
vc-git-registered still needs changes to deal with directories.
Alexandre Julliard will look at that when he gets a chance.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

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

  > > But I don't really want to check it in at this point without an explicit
  > > request from Stefan.
  > 
  > Removing calls to determine the backend (and replace them with backend
  > info propagated from elsewhere) is generally a good thing.

How about this one?  It removes an extra vc-backend call for each VC managed file.
And another one could be eliminated, but I am not 100% sure what this comment means:
      ;; Let the backend setup any buffer-local things he needs.
      (vc-call-backend (vc-backend buffer-file-name) 'find-file-hook))



--- vc-hooks.el.~1.276.~ Fri Jan 30 15:35:05 2009
+++ vc-hooks.el          Sun Mar 22 21:05:25 2009
@@ -761,12 +761,12 @@
     (define-key map [mode-line down-mouse-1] vc-menu-entry)
     map))
 
-(defun vc-mode-line (file)
+(defun vc-mode-line (file &optional backend-arg)
   "Set `vc-mode' to display type of version control for FILE.
 The value is set in the current buffer, which should be the buffer
 visiting FILE."
   (interactive (list buffer-file-name))
-  (let ((backend (vc-backend file)))
+  (let ((backend (or backend-arg (vc-backend file))))
     (if (not backend)
     (setq vc-mode nil)
       (let* ((ml-string (vc-call-backend backend 'mode-line-string file))
@@ -868,15 +868,16 @@
   "Function for `find-file-hook' activating VC mode if appropriate."
   ;; Recompute whether file is version controlled,
   ;; if user has killed the buffer and revisited.
-  (if vc-mode
-      (setq vc-mode nil))
+  (when vc-mode
+    (setq vc-mode nil))
+  (let (bk)
   (when buffer-file-name
     (vc-file-clearprops buffer-file-name)
     (add-hook 'mode-line-hook 'vc-mode-line nil t)
     (cond
-     ((with-demoted-errors (vc-backend buffer-file-name))
+     ((setq bk (with-demoted-errors (vc-backend buffer-file-name)))
       ;; Compute the state and put it in the modeline.
-      (vc-mode-line buffer-file-name)
+      (vc-mode-line buffer-file-name bk)
       (unless vc-make-backup-files
       ;; Use this variable, not make-backup-files,
       ;; because this is for things that depend on the file name.
@@ -909,7 +910,7 @@
                   (vc-find-file-hook))
                         (message
         "Warning: editing through the link bypasses version control")
-                   ))))))))
+                        )))))))))
 
 (add-hook 'find-file-hook 'vc-find-file-hook)
 

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Stefan Monnier]

>> > But I don't really want to check it in at this point without an explicit
>> > request from Stefan.
>> Removing calls to determine the backend (and replace them with backend
>> info propagated from elsewhere) is generally a good thing.

> How about this one?

Doesn't look too bad, tho "backend-arg" rings pretty poorly.  I think
this hints at the need to just use a single `backend' var (moved out of
the let into the arg) which is then defaulted via (unless backend (setq
backend (vc-backend file))).

> It removes an extra vc-backend call for each VC managed file.

Good.  Note that the issue is not performance but semantics: in most/all
of those cases, the behavior would be at best odd if the recomputed
backend were different from the caller's idea.

> And another one could be eliminated, but I am not 100% sure what this
> comment means:
>       ;; Let the backend setup any buffer-local things he needs.
>       (vc-call-backend (vc-backend buffer-file-name) 'find-file-hook))

The comment just means "run a hook so the backend can set things up".
Grep for "vc-.*-find-file-hook" to see for what it is currently used.
So yes, this should just use `backend' (which I'd prefer over `bk').


        Stefan

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

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

  > > But I don't really want to check it in at this point without an explicit
  > > request from Stefan.
  > 
  > Removing calls to determine the backend (and replace them with backend
  > info propagated from elsewhere) is generally a good thing.  I think your
  > patch looks good.

Here's another patch of the same kind: it removes vc-backend calls for
vc-stay-local-p.

OK or 23.2? 

Index: vc-cvs.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-cvs.el,v
retrieving revision 1.163
diff -u -3 -p -r1.163 vc-cvs.el
--- vc-cvs.el	5 Jan 2009 03:19:51 -0000	1.163
+++ vc-cvs.el	13 Apr 2009 16:43:24 -0000
@@ -216,7 +216,7 @@ See also variable `vc-cvs-sticky-date-fo
 
 (defun vc-cvs-state (file)
   "CVS-specific version of `vc-state'."
-  (if (vc-stay-local-p file)
+  (if (vc-stay-local-p file 'CVS)
       (let ((state (vc-file-getprop file 'vc-state)))
         ;; If we should stay local, use the heuristic but only if
         ;; we don't have a more precise state already available.
@@ -496,7 +496,7 @@ Will fail unless you have administrative
   ;; It's just the catenation of the individual logs.
   (vc-cvs-command
    buffer
-   (if (vc-stay-local-p files) 'async 0)
+   (if (vc-stay-local-p files 'CVS) 'async 0)
    files "log"))
 
 (defun vc-cvs-comment-history (file)
@@ -506,7 +506,7 @@ Will fail unless you have administrative
 (defun vc-cvs-diff (files &optional oldvers newvers buffer)
   "Get a difference report using CVS between two revisions of FILE."
   (let* ((async (and (not vc-disable-async-diff)
-		     (vc-stay-local-p files)))
+		     (vc-stay-local-p files 'CVS)))
 	 (invoke-cvs-diff-list nil)
 	 status)
     ;; Look through the file list and see if any files have backups
@@ -559,7 +559,7 @@ Will fail unless you have administrative
   "Execute \"cvs annotate\" on FILE, inserting the contents in BUFFER.
 Optional arg REVISION is a revision to annotate from."
   (vc-cvs-command buffer
-                  (if (vc-stay-local-p file)
+                  (if (vc-stay-local-p file 'CVS)
 		      'async 0)
                   file "annotate"
                   (if revision (concat "-r" revision)))
@@ -681,8 +681,9 @@ If UPDATE is non-nil, then update (resyn
 ;;; Miscellaneous
 ;;;
 
-(defalias 'vc-cvs-make-version-backups-p 'vc-stay-local-p
-  "Return non-nil if version backups should be made for FILE.")
+(defun vc-cvs-make-version-backups-p (file)
+  "Return non-nil if version backups should be made for FILE."
+  (vc-stay-local-p file 'CVS))
 
 (defun vc-cvs-check-headers ()
   "Check if the current file has any headers in it."
@@ -706,7 +707,8 @@ and that it passes `vc-cvs-global-switch
            (append vc-cvs-global-switches
                    flags))))
 
-(defalias 'vc-cvs-stay-local-p 'vc-stay-local-p)  ;Back-compatibility.
+(defun vc-cvs-stay-local-p (file)
+  (vc-stay-local-p file 'CVS))
 
 (defun vc-cvs-repository-hostname (dirname)
   "Hostname of the CVS server associated to workarea DIRNAME."
@@ -965,7 +967,7 @@ state."
 (defun vc-cvs-dir-status (dir update-function)
   "Create a list of conses (file . state) for DIR."
   ;; FIXME check all files in DIR instead?
-  (let ((local (vc-stay-local-p dir)))
+  (let ((local (vc-stay-local-p dir 'CVS)))
     (if (and local (not (eq local 'only-file)))
 	(vc-cvs-dir-status-heuristic dir update-function)
       (vc-cvs-command (current-buffer) 'async dir "-f" "status")
Index: vc-hooks.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-hooks.el,v
retrieving revision 1.277
diff -u -3 -p -r1.277 vc-hooks.el
--- vc-hooks.el	1 Apr 2009 15:42:12 -0000	1.277
+++ vc-hooks.el	13 Apr 2009 16:43:24 -0000
@@ -168,15 +168,15 @@ by these regular expressions."
   :version "23.1"
   :group 'vc)
 
-(defun vc-stay-local-p (file)
+(defun vc-stay-local-p (file &optional backend)
   "Return non-nil if VC should stay local when handling FILE.
 This uses the `repository-hostname' backend operation.
 If FILE is a list of files, return non-nil if any of them
 individually should stay local."
   (if (listp file)
-      (delq nil (mapcar 'vc-stay-local-p file))
-    (let* ((backend (vc-backend file))
-	   (sym (vc-make-backend-sym backend 'stay-local))
+      (delq nil (mapcar (lambda (arg) (vc-stay-local-p arg backend)) file))
+    (setq backend (or backend (vc-backend file)))
+    (let* ((sym (vc-make-backend-sym backend 'stay-local))
 	   (stay-local (if (boundp sym) (symbol-value sym) vc-stay-local)))
       (if (symbolp stay-local) stay-local
 	(let ((dirname (if (file-directory-p file)
Index: vc-svn.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-svn.el,v
retrieving revision 1.112
diff -u -3 -p -r1.112 vc-svn.el
--- vc-svn.el	10 Feb 2009 05:59:14 -0000	1.112
+++ vc-svn.el	13 Apr 2009 16:43:25 -0000
@@ -142,7 +142,7 @@ want to force an empty list of arguments
 
 (defun vc-svn-state (file &optional localp)
   "SVN-specific version of `vc-state'."
-  (setq localp (or localp (vc-stay-local-p file)))
+  (setq localp (or localp (vc-stay-local-p file 'SVN)))
   (with-temp-buffer
     (cd (file-name-directory file))
     (vc-svn-command t 0 file "status" (if localp "-v" "-u"))
@@ -176,7 +176,7 @@ want to force an empty list of arguments
 	     ;; FIXME are there other possible combinations?
 	     (cond ((eq state 'edited) (setq state 'needs-merge))
 		   ((not state) (setq state 'needs-update))))
-       (when state
+       (when (and state (not (string= "." filename)))
          (setq result (cons (list filename state) result)))))
     (funcall callback result)))
 
@@ -189,7 +189,7 @@ RESULT is a list of conses (FILE . STATE
   ;; calling synchronously (vc-svn-registered DIR) => calling svn status -v DIR
   ;; which is VERY SLOW for big trees and it makes emacs
   ;; completely unresponsive during that time.
-  (let* ((local (and nil (vc-stay-local-p dir)))
+  (let* ((local (and nil (vc-stay-local-p dir 'SVN)))
 	 (remote (or t (not local) (eq local 'only-file))))
     (vc-svn-command (current-buffer) 'async nil "status"
 		    (if remote "-u"))
@@ -470,7 +470,7 @@ or svn+ssh://."
 		  (vc-svn-command
 		   buffer
 		   'async
-		   ;; (if (and (= (length files) 1) (vc-stay-local-p file)) 'async 0)
+		   ;; (if (and (= (length files) 1) (vc-stay-local-p file 'SVN)) 'async 0)
 		   (list file)
 		   "log"
 		   ;; By default Subversion only shows the log up to the
@@ -502,7 +502,7 @@ or svn+ssh://."
 	      (list "--diff-cmd=diff" "-x"
 		    (mapconcat 'identity (vc-switches nil 'diff) " "))))
 	   (async (and (not vc-disable-async-diff)
-                       (vc-stay-local-p files)
+                       (vc-stay-local-p files 'SVN)
 		       (or oldvers newvers)))) ; Svn diffs those locally.
       (apply 'vc-svn-command buffer
 	     (if async 'async 0)
@@ -543,8 +543,9 @@ NAME is assumed to be a URL."
 ;;;
 
 ;; Subversion makes backups for us, so don't bother.
-;; (defalias 'vc-svn-make-version-backups-p 'vc-stay-local-p
-;;   "Return non-nil if version backups should be made for FILE.")
+;; (defun vc-svn-make-version-backups-p (file)
+;;   "Return non-nil if version backups should be made for FILE."
+;;  (vc-stay-local-p file 'SVN))
 
 (defun vc-svn-check-headers ()
   "Check if the current file has any headers in it."

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Stefan Monnier]

>> Removing calls to determine the backend (and replace them with backend
>> info propagated from elsewhere) is generally a good thing.  I think your
>> patch looks good.
> Here's another patch of the same kind: it removes vc-backend calls for
> vc-stay-local-p.
> OK or 23.2? 

Looks good, but I'd rather postpone it for 23.2.


        Stefan

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Miles Bader]

I tried your patch, and git directory commits seem to work well with
it applied!!

Thanks,

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2652: 23.0.91; wacky behavior of "v" command in *vc-dir* buffer

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Fri, Mar 13, 2009 at 4:27 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > > Thanks.
  > > The immediate cause of the problem is that
  > > (vc-git-registered "/tmp/zonk/")
  > > returns nil.
  > >
  > > vc-git-registered is called because vc-finish-log-entry does a
  > > `vc-call'.  `vc-call' tries to find the corresponding VC backend.
  > >
  > > Now, the VC backend should be know known at point when the log entry is
  > > started, so there should be no need for the vc-call, vc-call-backend
  > > could be used...
  > 
  > Is there anybody driving the design of the vc rewrite at this point?

BTW, IMO at this point there no real need for a vc rewrite.  There are
missing features, and maybe bugs, but the rewrite has put vc on a good
footing.