Re: [PATCH v2] git-clean: Display more accurate delete messages

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 19, 2012, 02:37 UTC
Message-ID: <7vy5gudaxx.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAKJhZwRPzrsnbnW_HgRTo86T6jqmm_osznDqpYo7pKO=cUaVDA@mail.gmail.com>

Zoltan Klinger <zoltan.klinger@gmail.com> writes:

Show 17 quoted lines

> Thanks for the feedback.
>
>> My reading of the above is that "lst" after sorting is expected to
>> have something like:
>>
>>         a/
>>         a/b/
>>         a/b/to-be-removed
>>         a/to-be-removed
>>
>> and we first show "a/", remember that prefix in "dir", not show
>> "a/b/" because it matches prefix, but still update the prefix to
>> "a/b/", not show "a/b/to-be-removed", and because "a/to-be-removed"
>> does not match the latest prefix, it is now shown.  Am I confused???
>
> No, it's a bug. The correct output should be just "a/". Thanks for
> pointing it out, I'm going to fix that.

I am not sure if the approach taken by the patch is an effective design to achieve what you are trying to do.

Imagine the code is told to "clean" (or "clean a") and is currentlly looking at "a/b" directory. If it cannot remove some paths under that directory, you know that you cannot abbreviate the result to "removed a/b" and have to report a/b/<paths you managed to remove> at that point. On the other hand, if you removed everything in that directory, you know you have only two possible outcomes regarding that directory in the final output:

 (1) You would say "removed a/b" if you failed to remove paths that
     are neighbours to that directory (e.g. "a/to-be-removed" may
     not go away for some reason), because you will also list
     "removed a/<other path>" next to it, and report that you
     couldn't remove "a/to-be-removed".  You will not report
     anything about "a/b/to-be-removed" in such a case; or

 (2) You would not even say "removed a/b" if you will successfully
     remove all other paths under "a/".

So in either case, if you managed to remove everything in "a/b", I do not see any reason to keep the list of successfully removed paths annd report them upwards. They will never be used by the caller that is looking at "a/", or its caller that is looking at the root level, will they?

On the other hand, if you failed to remove some paths under "a/b", before recursion leaves that directory, you know which paths to be reported as successful or failure, which means you can start producing output without waiting until the traversal touches the entire tree. That can be a huge latency win, which matters a lot in a large project.