Derrick Stolee <derrickstolee@xxxxxxxxxx> 于2023年1月6日周五 03:19写道：
>
> On 12/25/22 8:29 AM, ZheNing Hu via GitGitGadget wrote:
> > From: ZheNing Hu <adlternative@xxxxxxxxx>
>
> Sorry for the long wait in getting back to reviewing.
>
Ah, It's okay.
> > Sometimes when users use scalar to download a monorepo
> > with a long commit history, they want to check the
> > progress bar to know how long they still need to wait
> > during the fetch process, but scalar suppresses this
> > output by default.
> >
> > So let's check whether scalar stderr refer to a terminal,
> > if so, show progress, otherwise disable it.
>
> Thanks for updating to this strategy. I think it's an
> easier change to swallow. We can consider options like
> --progress, --verbose, or --quiet later while this
> change does the good work of showing terminal users
> helpful progress.
>
Yes, but I think something like `--quiet` is difficult to implement.
We cannot just add `--no-progress` or `--quiet` to the git checkout
for suppressing the progress. Because git checkout will not use it
to suppress the fetch progress, but only the checkout's progress.
> > + int full_clone = 0, single_branch = 0, show_progress = isatty(2);
>
> > - if ((res = run_git("fetch", "--quiet", "origin", NULL))) {
> > + if ((res = run_git("fetch", "--quiet",
> > + show_progress ? "--progress" : "--no-progress",
> > + "origin", NULL))) {
> > warning(_("partial clone failed; attempting full clone"));
> >
> > if (set_config("remote.origin.promisor") ||
> > @@ -508,7 +510,9 @@ static int cmd_clone(int argc, const char **argv)
> > goto cleanup;
> > }
> >
> > - if ((res = run_git("fetch", "--quiet", "origin", NULL)))
> > + if ((res = run_git("fetch", "--quiet",
> > + show_progress ? "--progress" : "--no-progress",
> > + "origin", NULL)))
> Implementation looks correct.
>
> > +test_expect_success TTY 'progress with tty' '
> > + enlistment=progress1 &&
> > +
> > + test_config -C to-clone uploadpack.allowfilter true &&
> > + test_config -C to-clone uploadpack.allowanysha1inwant true &&
> > +
> > + test_terminal env GIT_PROGRESS_DELAY=0 \
> > + scalar clone "file://$(pwd)/to-clone" "$enlistment" 2>stderr &&
>
> Thank you for creating this test!
>
> > + grep --count "Enumerating objects" stderr >actual &&
> > + echo 2 >expected &&
> > + test_cmp expected actual &&
>
> I think you could use "test_line_count = 2 actual" here.
>
Oh, good suggestion. I should also use grep without `--count` too.
> > + cleanup_clone $enlistment
> > +'
> > +
> > +test_expect_success 'progress without tty' '
> > + enlistment=progress2 &&
> > +
> > + test_config -C to-clone uploadpack.allowfilter true &&
> > + test_config -C to-clone uploadpack.allowanysha1inwant true &&
> > +
> > + scalar clone "file://$(pwd)/to-clone" "$enlistment" 2>stderr &&
> > + ! grep "Enumerating objects" stderr &&
> > + ! grep "Updating files" stderr &&
>
> Here, it would be good to still have the GIT_PROGRESS_DELAY=0
> environment variable on the 'scalar clone' command to be sure
> we are not getting these lines because progress is turned off
> and not because it's running too quickly.
>
OK, that makes sense.
> > + cleanup_clone $enlistment
> > +'
> > test_done
>
> A nit: there should be an empty line between the end quote of
> the last test and "test_done".
>
> Thanks,
> -Stolee
Thanks,
-ZheNing Hu
