Re: [PATCH v1] convert: display progress for filtered objects that have been delayed

> On 24 Aug 2017, at 21:40, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>> In 2841e8f ("convert: add "status=delayed" to filter process protocol",
>> 2017-06-30) we taught the filter process protocol to delayed responses.
>> These responses are processed after the "Checking out files" phase.
>> If the processing takes noticeable time, then the user might think Git
>> is stuck.
>> 
>> Display the progress of the delayed responses to let the user know that
>> Git is still processing objects. This works very well for objects that
>> can be filtered quickly. If filtering of an individual object takes
>> noticeable time, then the user might still think that Git is stuck.
>> However, in that case the user would at least know what Git is doing.
>> 
>> It would be technical more correct to display "Checking out files whose
>> content filtering has been delayed". For brevity we only print
>> "Filtering content".
>> 
>> The finish_delayed_checkout() call was moved below the stop_progress()
>> call in unpack-trees.c to ensure that the "Checking out files" progress
>> is properly stopped before the "Filtering content" progress starts in
>> finish_delayed_checkout().
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> Suggested-by: Taylor Blau <me@ttaylorr.com>
>> ---
> 
> Makes sense.  The only thing that made me wonder was if we want the
> change in unpack-trees.c in this patch.  After all, the procedure to
> finish up the delayed checkout _is_ a part of the work need to be
> done to populate the working tree files, so stopping the progress
> before feels somewhat wrong at the phylosophical level.
> 
> I think our output cannot express nested progress bars, and I think
> that is the reason why this patch tweaks unpack-trees.c; so I am
> fine with the end result (and that is why I said "made me wonder
> was", not "makes me wonder", the latter would imply "this we might
> want fix before applying", but I do not think we want to change
> anything this patch does to unpack-trees.c in this case).
> 
> The delayed progress API is being simplified so I'll probably do a
> bit of evil merge while merging this to 'pu'.
> 
> Thanks.