Re: [PATCH v3] scalar: show progress if stderr refer to a terminal

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 13, 2023, 19:52 UTC
Message-ID: <xmqqh6wu2ow9.fsf@gitster.g>
In-Reply-To: <9d8e38fd-f001-5aa5-ab78-cc6d314df09a@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 25 quoted lines

> On 1/11/2023 8:14 AM, ZheNing Hu via GitGitGadget wrote:
>> From: ZheNing Hu <adlternative@gmail.com>
>
>> Range-diff vs v2:
>
>>      -+test_expect_success 'progress without tty' '
>>      ++test_expect_success TTY 'progress without tty' '
>
> I think this addition of the TTY prerequisite is not necessary...
>
>> +test_expect_success TTY 'progress without tty' '
>> +	enlistment=progress2 &&
>> +
>> +	test_config -C to-clone uploadpack.allowfilter true &&
>> +	test_config -C to-clone uploadpack.allowanysha1inwant true &&
>> +
>> +	GIT_PROGRESS_DELAY=0 scalar clone "file://$(pwd)/to-clone" "$enlistment" 2>stderr &&
>> +	! grep "Enumerating objects" stderr &&
>> +	! grep "Updating files" stderr &&
>> +	cleanup_clone $enlistment
>> +'
>
> ...because the test doesn't use the environment details for
> mimicing a TTY. The point is that stderr is redirected to a
> file and isatty(2) would report false.

Yup, the prerequisite was uttering misleading. I may queue it with local tweaks, but if I forget please send in an update.

Thanks.

Show 5 quoted lines

> I don't think this is worth a re-roll, though, so I'm happy
> with this version.
>
> Thanks,
> -Stolee