Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add

From: Matthieu Moy <matthieu.moy@imag.fr>
Date: Jul 9, 2007, 09:45 UTC
Message-ID: <vpqsl7xkj0j.fsf@bauges.imag.fr>
In-Reply-To: <Pine.LNX.4.64.0707082240510.4248@racer.site>

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

Show 12 quoted lines

> Hi,
>
> On Sun, 8 Jul 2007, Matthieu Moy wrote:
>
>> I'm not sure whether this is really wrong. The things git should
>> really care about are the index and the repository itself, and the
>> proposed behavior is consistant regarding that (either remove all
>> files from the index, or remove none).
>
> Well, I think it is wrong for the same reason as it is wrong to apply the 
> changes to _any_ file when one would fail.  And since "git apply" shares 
> my understanding, I think "git rm" should, too.

OK, let's say I'm convinced ;-).

Show 11 quoted lines

>> > I suspect that this case does never fail. 0 means success for 
>> > remove_file().  Not good.  You should at least have a way to ensure that 
>> > it removed the files from the working tree from a script.  Otherwise there 
>> > is not much point in returning a value to begin with.
>> 
>> I've changed it to have exit_status = 1 if git-rm aborted before
>> starting, and 2 if git-rm skiped some file removals (and of course, 0
>> if everything is done as expected).
>
> Oh, so you do not take the return value of this function to determine if 
> it has or has not done something with the files?  That's a bit confusing.

I did, but previously, I kept the code that "die()"s if the first call to remove_file() "fails". In remove_file_maybe(), not removing a file because it's not sure it's safe to delete it is not a failure, so I had to put a "return 0;" here to avoid the fatal error. My first patch had return status !=0 if we tried to remove the file, and it failed. I changed that.

> I meant to complain about your OP, but this time it is even worse.  The 
> best way to guarantee that a patch gets lost in a thread is to move it _at 
> the end_ of a reply.

I had posted the patch for info, but I did expect this one to get lost, since it's definitely not complete.

I'll post an updated patch with a testcase and an appropriate subject line within a few days (I don't have time right now).

Thanks,

-- 
Matthieu