git-add--interactive works only in top level

git-add--interactive works only in top level

[Reid Barton]

When the working directory is not the top of the working tree, git- 
add--interactive fails silently and confusingly.  In this example,  
the working tree is rooted at ~/sandbox/foo and ~/sandbox/foo/bar/x  
is a file that's been edited.

rwbarton@homothety:~/sandbox/foo/bar$ git-add--interactive
            staged     unstaged path
   1:    unchanged        +1/-0 bar/x

*** Commands ***
   1: status       2: update       3: revert       4: add untracked
   5: patch        6: diff         7: quit         8: help
What now> 5
            staged     unstaged path
   1:    unchanged        +1/-0 bar/x
Patch update> 1

*** Commands ***
   1: status       2: update       3: revert       4: add untracked
   5: patch        6: diff         7: quit         8: help
What now>

Rather than returning to the main menu, git-add--interactive should  
have showed me a list of chunks.  It seems that the list of files is  
relative to the working tree root (which is sensible) but when git- 
add--interactive invokes git-diff-files it does not take this into  
account.  Perhaps git-diff-files should also complain when invoked as

git-diff-files -- non-existent-file-name

so it wouldn't have taken me half an hour to track this down.

Regards,
Reid Barton

Re: git-add--interactive works only in top level

[Junio C Hamano]

Reid Barton <rwbarton@MIT.EDU> writes:

> When the working directory is not the top of the working tree, git-
> add--interactive fails silently and confusingly.

Sheesh, you got me worried.

This is a non issue; git-add--interactive is not for direct end user
consumption.  It relies on being run from git-add wrapper, which does
cdup to the top of the work tree before launching add--interactive
helper.

Re: git-add--interactive works only in top level

[Reid Barton]

On Dec 4, 2007, at 12:44 AM, Junio C Hamano wrote:
> Sheesh, you got me worried.
>
> This is a non issue; git-add--interactive is not for direct end user
> consumption.  It relies on being run from git-add wrapper, which does
> cdup to the top of the work tree before launching add--interactive
> helper.

Ah, OK.  Would it not be easy then to add a check to the beginning of  
git-add--interactive to verify that it is indeed being run from the  
top of the work tree?  A user may well discover git-add--interactive  
before git-add --interactive (e.g. by shell completion) and if git- 
add--interactive is going to fail, it should do so right away so as  
to not confuse the user.

I understand that programs such as git-add--interactive will be moved  
out of the executable path not too long from now, which will also  
ameliorate the situation.

Regards,
Reid Barton

Re: git-add--interactive works only in top level

[Junio C Hamano]

Reid Barton <rwbarton@MIT.EDU> writes:

> I understand that programs such as git-add--interactive will be moved
> out of the executable path not too long from now, which will also
> ameliorate the situation.

Honestly, there is nothing to ameliorate.  We do not even document
git-add--interactive on purpose.

Once I saw somebody who somehow got a root account on a shared UNIX box
and tryed running everything he found under /sbin one after another
without understanding what he was doing.  Needless to say, the box did
not last too long.  Somehow that "tab completion" comment reminds of
him.

Re: git-add--interactive works only in top level

[Wincent Colaiuta]

El 4/12/2007, a las 7:22, Junio C Hamano escribió:

> Reid Barton <rwbarton@MIT.EDU> writes:
>
>> I understand that programs such as git-add--interactive will be moved
>> out of the executable path not too long from now, which will also
>> ameliorate the situation.
>
> Honestly, there is nothing to ameliorate.  We do not even document
> git-add--interactive on purpose.
>
> Once I saw somebody who somehow got a root account on a shared UNIX  
> box
> and tryed running everything he found under /sbin one after another
> without understanding what he was doing.  Needless to say, the box did
> not last too long.  Somehow that "tab completion" comment reminds of
> him.

Here he have two options: blame the user for his/her stupidity, or  
look at this a UI problem and try to make the UI "idiot proof".

In this particular case I see no impediment to favouring the second  
option, because we're not talking about making changes that would make  
Git less powerful or useful for "non-idiots", "power users" or "real  
men" (whatever you want to call them). In other words, we are not  
talking about "dumbing down" Git for the sake of the ignorant. This is  
an opportunity to polish the UI in the same way that we polish the  
internal pack format.

And while it's not a good idea to login as root and try everything  
under "/sbin" without knowing what it does, stumbling across "git-add-- 
interactive" via tab completion is easier to understand. It's an easy  
enough mistake to make and it would be nice if we shielded the user  
from making it.

I think it is (and always was) an error to expose things like git-add-- 
interactive which are fundamentally implementation details *internal*  
to Git. The user shouldn't even know that it exists. It's one thing to  
have a bunch of high-level commands (porcelain) mixed together with  
low-level ones (plumbing) in the user's PATH, but it's another  
entirely to also stick internal implementation stuff which should  
*never* be directly executed in there too. At least in the case of  
plumbing there are reasons why you might sometimes directly execute  
it, but that's not the case with stuff like git-add--interactive.

So I do think there is something to ameliorate, and moving this stuff  
out of the PATH will do exactly that.

Cheers,
Wincent

Re: git-add--interactive works only in top level

[Johannes Schindelin]

Hi,

On Tue, 4 Dec 2007, Wincent Colaiuta wrote:

> El 4/12/2007, a las 7:22, Junio C Hamano escribi?:
> 
> > Reid Barton <rwbarton@MIT.EDU> writes:
> > 
> > > I understand that programs such as git-add--interactive will be 
> > > moved out of the executable path not too long from now, which will 
> > > also ameliorate the situation.
> > 
> > Honestly, there is nothing to ameliorate.  We do not even document 
> > git-add--interactive on purpose.
> > 
> > Once I saw somebody who somehow got a root account on a shared UNIX 
> > box and tryed running everything he found under /sbin one after 
> > another without understanding what he was doing.  Needless to say, the 
> > box did not last too long.  Somehow that "tab completion" comment 
> > reminds of him.
> 
> Here he have two options: blame the user for his/her stupidity, or look 
> at this a UI problem and try to make the UI "idiot proof".
> 
> In this particular case I see no impediment to favouring the second 
> option, because we're not talking about making changes that would make 
> Git less powerful or useful for "non-idiots", "power users" or "real 
> men" (whatever you want to call them). In other words, we are not 
> talking about "dumbing down" Git for the sake of the ignorant. This is 
> an opportunity to polish the UI in the same way that we polish the 
> internal pack format.

You know, without patches you will not convince me ;-)

Ciao,
Dscho

Re: git-add--interactive works only in top level

[Wincent Colaiuta]

El 4/12/2007, a las 12:48, Johannes Schindelin escribió:

> You know, without patches you will not convince me ;-)

A number of patches have already been submitted (by Nguyễn Thái  
Ngọc Duy, I believe); you know, the "Move all dashed form git  
commands to libexecdir" thread and its precursors. I was just arguing  
in support of the rationale behind them.

Cheers,
Wincent

Re: git-add--interactive works only in top level

[Jakub Narebski]

Wincent Colaiuta <win@wincent.com> writes:

> El 4/12/2007, a las 12:48, Johannes Schindelin escribió:
> 
> > You know, without patches you will not convince me ;-)
> 
> A number of patches have already been submitted (by Nguy…n Tha  
> Ngc Duy, I believe); you know, the "Move all dashed form git  
> commands to libexecdir" thread and its precursors. I was just arguing  
> in support of the rationale behind them.

I think creating separate libexecdir, and moving there _helpers_ first
(i.e. command which are neither plumbing, nor porcelain, but are
meant to be called _only_ by other commands) would be a good idea,
perhaps even pre 1.5.4.

By the way, if (when) dashed form of commands would get moved to
libexecdir, the INSTALL fragment below talking about removin wrapper
script would have to be changed. But does anyone use GNU Interactive
Tools anymore (IIRC the package name was changed to gitfm or sth.)

<quote src="INSTALL">
 - git normally installs a helper script wrapper called "git", which
   conflicts with a similarly named "GNU interactive tools" program.

   Tough.  Either don't use the wrapper script, or delete the old GNU
   interactive tools.  None of the core git stuff needs the wrapper,
   it's just a convenient shorthand and while it is documented in some
   places, you can always replace "git commit" with "git-commit"
   instead.

   But let's face it, most of us don't have GNU interactive tools, and
   even if we had it, we wouldn't know what it does.  I don't think it
   has been actively developed since 1997, and people have moved over to
   graphical file managers.
</quote>

-- 
Jakub Narebski

Re: git-add--interactive works only in top level

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Here he have two options: blame the user for his/her stupidity, or look 
>> at this a UI problem and try to make the UI "idiot proof".
>> 
>> In this particular case I see no impediment to favouring the second 
>> option, because we're not talking about making changes that would make 
>> Git less powerful or useful for "non-idiots", "power users" or "real 
>> men" (whatever you want to call them). In other words, we are not 
>> talking about "dumbing down" Git for the sake of the ignorant. This is 
>> an opportunity to polish the UI in the same way that we polish the 
>> internal pack format.
>
> You know, without patches you will not convince me ;-)

Honestly, I do not want a patch for that to git-add--interactive.

Once WIncent's and Dan's UI enhancements to it becomes stable (not
implementation-wise but more interface- and user-experience-wise),
rewriting it in C should not be too much of a hassle.  When that
happens, it won't be a standalone program but will be a function
builtin-add.c::interactive_add() will call into and we won't have this
problem at that point.  If somebody does do a patch, the time is better
spent there instead.