bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Miles Bader]

[Note: I have Dan's patch for fixing whole-directory vc-dir commit in
Git applied, though I don't think it should affect the following, as it
relates to final commit.]

If I have a vc-dir buffer showing a tree where the only entries are new
files (not yet registered with source-control):

   VC backend : Git
   Working dir: /tmp/zonk/
   Branch     : master

                            ./
        unregistered        llll
        unregistered        oiuoiu

Then hitting "v" on the first line of the buffer acts strangely -- I'd
expect it to simply register all these files with source control, but in
fact, it simply appears to do nothing.

There are actually several behaviors (though it always does nothing in
the end).  If this is the first time I've tried to do this, it just
displays a message like "Registering (/tmp/zonk/)... done" [where
"/tmp/zonk2" is the root of the working directory in question] but
doesn't seem to really do it.

If I then try the "v" command again, it will first show the message:

   Previous master file has vanished.  Make a new one? (y or n) 

If I enter "n" it just gives an error and aborts.

If I type "y", then it will either display a "Registering
(/tmp/zonk/)... done" message like the initial time (and again have no
real result), or show a *vc-log* buffer to prompt for an "initial
comment"; in the latter case, when I then hit C-c C-c to continue, the
*vc-log* buffer goes away -- but nothing further appears to happen.
[I'm not sure what causes it to open a *vc-log* buffer or not ... in my
initial tests it did, but when I tried again with emacs -Q, it didn't.]

Anyway, the end result is always the same, no registered files.


To reproduce:

  (1) Execute the following shell script to create a test repo in
      "/tmp/zonk2":

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

  (2) Start emacs:  HOME=/tmp emacs -Q -nw

  (3) Start vc-dir on the test repo:  C-x v d /tmp/zonk2 RET

  (4) Without moving the cursor, try to register the new files:  v
  
  (5) It will exhibit one of the faulty behaviors described above,
      resulting in no file registration being done.

Thanks,

-Miles


If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
If you would like to further debug the crash, please read the file
/usr/local/share/emacs/23.0.91/etc/DEBUG for instructions.


In GNU Emacs 23.0.91.11 (x86_64-unknown-linux-gnu, GTK+ Version 2.15.5)
 of 2009-03-13 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: Message

Minor modes in effect:
  diff-auto-refine-mode: t
  shell-dirtrack-mode: t
  mml-mode: t
  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
  abbrev-mode: t

Recent input:
SPC y o u SPC n <backspace> g i v e <escape> h s p 
e c i f y SPC ( a n d SPC l e a v e s SPC a n y SPC 
o t h e r s SPC a l o n e ) <return> - C-x d C-x b 
* G r SPC <return> K g C-c C-SPC C-c C-SPC C-c C-SPC 
K g n n n n n n n p p p p p p SPC SPC q n n SPC q n 
= SPC n n n q C-x b * v c SPC - d SPC r SPC / SPC <return> 
C-n C-n C-n C-n C-n C-n C-p C-n m C-u C-p C-p v w a 
c k y <return> C-c C-c x v z o i n k <return> c C-c 
C-c C-c C-g C-a C-n C-p C-p v y z o i n k <return> 
C-c C-c g g g C-h f C-g C-h e <escape> > C-x d C-n 
C-n C-n v y a o i n <return> C-c C-c C-u C-p c v y 
v o i n <return> C-c C-c C-n C-a C-n C-n C-n v C-c 
C-c C-u C-p C-p v p l o o n <return> C-c C-c C-a C-a 
C-x C-v p l C-g C-x C-v n e w f SPC 4 <return> l <return> 
<return> C-x C-s C-x k <return> C-u C-p v k o k <return> 
C-c C-c x C-x C-v l l l l <return> l k j <return> C-x 
C-s C-x k <return> C-p C-p v n C-a C-x h <escape> w 
C-x b * s e SPC m SPC SPC < 2 SPC <return> <escape> 
x r e p o SPC r SPC e SPC <return>

Recent messages:
Checking in /tmp/zonk/...done
(New file)
Saving file /tmp/zonk/llll...
Wrote /tmp/zonk/llll
xding
Previous master file has vanished.  Make a new one? (y or n) 
xding
vc-register: Aborted
Mark set [2 times]
Making completion list... [2 times]

-- 
Zeal, n. A certain nervous disorder afflicting the young and inexperienced.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > [Note: I have Dan's patch for fixing whole-directory vc-dir commit in
  > Git applied, though I don't think it should affect the following, as it
  > relates to final commit.]
  > 
  > If I have a vc-dir buffer showing a tree where the only entries are new
  > files (not yet registered with source-control):
  > 
  >    VC backend : Git
  >    Working dir: /tmp/zonk/
  >    Branch     : master
  > 
  >                             ./
  >         unregistered        llll
  >         unregistered        oiuoiu
  > 
  > Then hitting "v" on the first line of the buffer acts strangely -- I'd
  > expect it to simply register all these files with source control, but in
  > fact, it simply appears to do nothing.
  > 
  > There are actually several behaviors (though it always does nothing in
  > the end).  If this is the first time I've tried to do this, it just
  > displays a message like "Registering (/tmp/zonk/)... done" [where
  > "/tmp/zonk2" is the root of the working directory in question] but
  > doesn't seem to really do it.

This is a vc-git.el problem, vc-git-register does not register the files.
Try the same thing with, say, mercurial and it work.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > [Note: I have Dan's patch for fixing whole-directory vc-dir commit in
  > Git applied, though I don't think it should affect the following, as it
  > relates to final commit.]
  > 
  > If I have a vc-dir buffer showing a tree where the only entries are new
  > files (not yet registered with source-control):
  > 
  >    VC backend : Git
  >    Working dir: /tmp/zonk/
  >    Branch     : master
  > 
  >                             ./
  >         unregistered        llll
  >         unregistered        oiuoiu
  > 
  > Then hitting "v" on the first line of the buffer acts strangely -- I'd
  > expect it to simply register all these files with source control, but in
  > fact, it simply appears to do nothing.
  > 
  > There are actually several behaviors (though it always does nothing in
  > the end).  If this is the first time I've tried to do this, it just
  > displays a message like "Registering (/tmp/zonk/)... done" [where
  > "/tmp/zonk2" is the root of the working directory in question] but
  > doesn't seem to really do it.
  > 
  > If I then try the "v" command again, it will first show the message:
  > 
  >    Previous master file has vanished.  Make a new one? (y or n) 
  > 
  > If I enter "n" it just gives an error and aborts.
  > 
  > If I type "y", then it will either display a "Registering
  > (/tmp/zonk/)... done" message like the initial time (and again have no
  > real result), or show a *vc-log* buffer to prompt for an "initial
  > comment"; in the latter case, when I then hit C-c C-c to continue, the
  > *vc-log* buffer goes away -- but nothing further appears to happen.
  > [I'm not sure what causes it to open a *vc-log* buffer or not ... in my
  > initial tests it did, but when I tried again with emacs -Q, it didn't.]
  > 
  > Anyway, the end result is always the same, no registered files.
  > 
  > 
  > To reproduce:
  > 
  >   (1) Execute the following shell script to create a test repo in
  >       "/tmp/zonk2":
  > 
  > #!/bin/sh
  > cd /tmp
  > rm -rf zonk2
  > mkdir zonk2
  > cd zonk2
  > git init
  > echo plugh > ppling
  > git add .
  > git commit -m'Init' -a
  > echo Fnord >> newf1
  > echo Chnevy >> newf2

Given that you are familiar with git, the function to fix to get this
working is the one liner `vc-git-register':

(defun vc-git-register (files &optional rev comment)
  "Register FILE into the git version-control system."
  (vc-git-command nil 0 files "update-index" "--add" "--"))


just add the correct command(s) to run to vc-git-command, and it should
work.

In case above vc-git-register is called like this:

(vc-git-register "/tmp/zonk2/")

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Miles Bader]

On Sat, Mar 21, 2009 at 1:15 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
> Given that you are familiar with git, the function to fix to get this
> working is the one liner `vc-git-register':
>
> (defun vc-git-register (files &optional rev comment)
>  "Register FILE into the git version-control system."
>  (vc-git-command nil 0 files "update-index" "--add" "--"))
...
> (vc-git-register "/tmp/zonk2/")

The obvious solution is to use "add" instead of "update-index --add".

[I tested it, it works.]

However, it'd be nice to ask whoever wrote this function originally
whether there was a reason for using "update-index" instead of "add",
given that vc.el apparently expects higher-level level functionality
(recursion) from the register hook.  I suppose update-index is the
more conservative choice, since it's officially "plumbing", but I'm
not aware of any problems with using the higher-level git-add in this
case.  [The git backend already uses "add" in other places too.]

Also, I wonder:

  (1) Given that vc-dir already has a list of files presented in the
directory listing, how come it doesn't give the backend that list,
instead of just the directory (having the backend do the recursion
seems more likely to yield unexpected results)?

   (2) Perhaps vc-dir it should do a "revert-buffer" (or something)
afterwards when it operations on a subdirectory or the whole
directory, as any operation with the backend doing the recursion may
change something other than what is displayed.  This is easy to see if
their were unregistered subdirectories with files -- doing "v" to
register a subdirectory adds all files in that directory, which vc-dir
hadn't displayed before, so currently "v" doesn't properly show them
afterwards (doing "g" shows them).

thanks,

-miles

-- 
Do not taunt Happy Fun Ball.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Sat, Mar 21, 2009 at 1:15 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > > Given that you are familiar with git, the function to fix to get this
  > > working is the one liner `vc-git-register':
  > >
  > > (defun vc-git-register (files &optional rev comment)
  > >  "Register FILE into the git version-control system."
  > >  (vc-git-command nil 0 files "update-index" "--add" "--"))
  > ...
  > > (vc-git-register "/tmp/zonk2/")
  > 
  > The obvious solution is to use "add" instead of "update-index --add".
  > 
  > [I tested it, it works.]
  > 
  > However, it'd be nice to ask whoever wrote this function originally
  > whether there was a reason for using "update-index" instead of "add",

I'd assume it would be the vc-git.el author.

  > given that vc.el apparently expects higher-level level functionality
  > (recursion) from the register hook.  I suppose update-index is the
  > more conservative choice, since it's officially "plumbing", but I'm
  > not aware of any problems with using the higher-level git-add in this
  > case.  [The git backend already uses "add" in other places too.]
  > 
  > Also, I wonder:
  > 
  >   (1) Given that vc-dir already has a list of files presented in the
  > directory listing, how come it doesn't give the backend that list,
  > instead of just the directory (having the backend do the recursion
  > seems more likely to yield unexpected results)?

When the point is on the "./" entry (or before), and nothing is selected
it means that you are asking for an action to be performed on the
directory, that is gets sent to the VC command.
If you want a list of files to be sent, mark a list of files and that is
what will be sent.

  >    (2) Perhaps vc-dir it should do a "revert-buffer" (or something)
  > afterwards when it operations on a subdirectory or the whole
  > directory, as any operation with the backend doing the recursion may
  > change something other than what is displayed.  This is easy to see if
  > their were unregistered subdirectories with files -- doing "v" to
  > register a subdirectory adds all files in that directory, which vc-dir
  > hadn't displayed before, so currently "v" doesn't properly show them
  > afterwards (doing "g" shows them).

It does, but there might be other things not working correctly with git,
try with hg and you will see it working correctly.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Miles Bader]

On Sat, Mar 21, 2009 at 11:54 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
>  >    (2) Perhaps vc-dir it should do a "revert-buffer" (or something)
>  > afterwards when it operations on a subdirectory or the whole
>  > directory, as any operation with the backend doing the recursion may
>  > change something other than what is displayed.  This is easy to see if
>  > their were unregistered subdirectories with files -- doing "v" to
>  > register a subdirectory adds all files in that directory, which vc-dir
>  > hadn't displayed before, so currently "v" doesn't properly show them
>  > afterwards (doing "g" shows them).
>
> It does, but there might be other things not working correctly with git,
> try with hg and you will see it working correctly.

It does what?  A revert?  Or always updates the display correctly in
the subdir case without a revert?  If it's the latter, what mechanism
does the backend use to communicate to vc-dir that more stuff has
shown up?  [or does the backend just do a revert itself?]

Thanks,

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Sat, Mar 21, 2009 at 11:54 AM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > >  >    (2) Perhaps vc-dir it should do a "revert-buffer" (or something)
  > >  > afterwards when it operations on a subdirectory or the whole
  > >  > directory, as any operation with the backend doing the recursion may
  > >  > change something other than what is displayed.  This is easy to see if
  > >  > their were unregistered subdirectories with files -- doing "v" to
  > >  > register a subdirectory adds all files in that directory, which vc-dir
  > >  > hadn't displayed before, so currently "v" doesn't properly show them
  > >  > afterwards (doing "g" shows them).
  > >
  > > It does, but there might be other things not working correctly with git,
  > > try with hg and you will see it working correctly.
  > 
  > It does what?  

vc-register creates a call to vc-start-logentry
vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer
vc-resynch-buffer updates both the buffers that might have been
affected by the VC command and the vc-dir display (this one via the 
vc-dir-resynch-file call).

  > A revert?  Or always updates the display correctly in
  > the subdir case without a revert?  If it's the latter, what mechanism
  > does the backend use to communicate to vc-dir that more stuff has
  > shown up?  [or does the backend just do a revert itself?]



  > Thanks,
  > 
  > -Miles
  > 
  > -- 
  > Do not taunt Happy Fun Ball.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Miles Bader]

On Sat, Mar 21, 2009 at 12:50 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
>  > > It does, but there might be other things not working correctly with git,
>  > > try with hg and you will see it working correctly.
>  >
>  > It does what?
>
> vc-register creates a call to vc-start-logentry
> vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer
> vc-resynch-buffer updates both the buffers that might have been
> affected by the VC command and the vc-dir display (this one via the
> vc-dir-resynch-file call).

Oh, incidentally -- that's another problem with the vc register stuff,
it prompts for a log message, which is useless for git (is it useful
for any non-cvs system?).  Is there a way for the backend to suppress
that behavior?

Thanks,

-Miles


-- 
Do not taunt Happy Fun Ball.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

Miles Bader <miles@gnu.org> writes:

  > On Sat, Mar 21, 2009 at 12:50 PM, Dan Nicolaescu <dann@ics.uci.edu> wrote:
  > >  > > It does, but there might be other things not working correctly with git,
  > >  > > try with hg and you will see it working correctly.
  > >  >
  > >  > It does what?
  > >
  > > vc-register creates a call to vc-start-logentry
  > > vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer
  > > vc-resynch-buffer updates both the buffers that might have been
  > > affected by the VC command and the vc-dir display (this one via the
  > > vc-dir-resynch-file call).
  > 
  > Oh, incidentally -- that's another problem with the vc register stuff,
  > it prompts for a log message, which is useless for git (is it useful
  > for any non-cvs system?).  

Probably not...

  > Is there a way for the backend to suppress that behavior?

I've never seen this happen, so I don't know of the top of my head how
to avoid it.  Can you describe what you do so that I can reproduce it?

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Miles Bader]

>  > Oh, incidentally -- that's another problem with the vc register stuff,
>  > it prompts for a log message, which is useless for git (is it useful
>  > for any non-cvs system?).
>
> Probably not...
>
>  > Is there a way for the backend to suppress that behavior?
>
> I've never seen this happen, so I don't know of the top of my head how
> to avoid it.  Can you describe what you do so that I can reproduce it?

Hmm, actually it seems to be my configuration -- I have
`vc-initial-comment' customized to `t'.

Perhaps it should be ignored for backends where it's pointless, but
for now I'll just change my configuration ... :-)

-Miles

-- 
Do not taunt Happy Fun Ball.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Stefan Monnier]

> vc-register creates a call to vc-start-logentry
> vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer

BTW, this trip via vc-start-logentry should be removed.  It's a remnant
from old times when vc-register had an opportunity to provide a message
linked to the file's addition, but nowadays it's just extra baggage that
makes the code less readable.


        Stefan

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

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

  > > vc-register creates a call to vc-start-logentry
  > > vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer
  > 
  > BTW, this trip via vc-start-logentry should be removed.  It's a remnant
  > from old times when vc-register had an opportunity to provide a message
  > linked to the file's addition, but nowadays it's just extra baggage that
  > makes the code less readable.

Note on this added to vc.el TODO.

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Dan Nicolaescu]

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

  > > vc-register creates a call to vc-start-logentry
  > > vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer
  > 
  > BTW, this trip via vc-start-logentry should be removed.  It's a remnant
  > from old times when vc-register had an opportunity to provide a message
  > linked to the file's addition, but nowadays it's just extra baggage that
  > makes the code less readable.

Would such a change be acceptable for 23.1?  It's straight forward to do...

bug#2678: 23.0.91; vc-next-action in vc-dir acts strangely when only adds are necessary

[Stefan Monnier]

>> > vc-register creates a call to vc-start-logentry
>> > vc-start-logentry calls vc-finish-logentry which calls vc-resynch-buffer
>> BTW, this trip via vc-start-logentry should be removed.  It's a remnant
>> from old times when vc-register had an opportunity to provide a message
>> linked to the file's addition, but nowadays it's just extra baggage that
>> makes the code less readable.
> Would such a change be acceptable for 23.1?  It's straight forward to do...

No, we want to stick to bug-fixes only.


        Stefan