Taylor Blau <me@xxxxxxxxxxxx> 于2022年12月8日周四 06:10写道：
>
> On Wed, Dec 07, 2022 at 06:10:56PM +0000, ZheNing Hu via GitGitGadget wrote:
> > So add `[--verbose| -v]` to scalar clone, to enable
> > fetch's output.
>
> Seems reasonable.
>
> > @@ -84,6 +84,11 @@ cloning. If the HEAD at the remote did not point at any branch when
> > A sparse-checkout is initialized by default. This behavior can be
> > turned off via `--full-clone`.
> >
> > +-v::
> > +--verbose::
> > + When scalar executes `git fetch`, `--quiet` is used by default to
> > + suppress the output of fetch, use verbose mode for cancel this.
> > +
>
> This description may be exposing a few too many implementation details
> for our liking. E.g., scalar happens to use `git fetch`, but it might
> not always. That is probably academic, but a more practical reason to do
> some hiding here might just be that it's unnecessary detail to expose in
> our documentation.
>
Hmmm. There are two steps to downloading data from scalar clone:
the first step is to let "git fetch partial clone" to download commits,
trees, tags, and the second step is download the blobs corresponding
to the top-level files of the repository during git checkout. So I'm not sure
if I should mention "fetch" here, since the progress bar for the "checkout"
step is able to be displayed.
> Perhaps something like:
>
> -v::
> --verbose::
> Enable more verbose output when cloning a repository.
>
Just mentioning "clone" is fine... But I'm not sure if users will be
confused, why they will "more verbose" instead of two options
"full verbose" or "not verbose".
> Or something simple like that.
>
> > List
> > ~~~~
> >
> > diff --git a/scalar.c b/scalar.c
> > index 6c52243cdf1..b1d4504d136 100644
> > --- a/scalar.c
> > +++ b/scalar.c
> > @@ -404,7 +404,7 @@ void load_builtin_commands(const char *prefix, struct cmdnames *cmds)
> > static int cmd_clone(int argc, const char **argv)
> > {
> > const char *branch = NULL;
> > - int full_clone = 0, single_branch = 0;
> > + int full_clone = 0, single_branch = 0, verbosity = 0;
> > struct option clone_options[] = {
> > OPT_STRING('b', "branch", &branch, N_("<branch>"),
> > N_("branch to checkout after clone")),
> > @@ -413,6 +413,7 @@ static int cmd_clone(int argc, const char **argv)
> > OPT_BOOL(0, "single-branch", &single_branch,
> > N_("only download metadata for the branch that will "
> > "be checked out")),
> > + OPT__VERBOSITY(&verbosity),
> > OPT_END(),
> > };
> > const char * const clone_usage[] = {
>
> Looking good.
>
> > @@ -499,7 +500,9 @@ static int cmd_clone(int argc, const char **argv)
> > if (set_recommended_config(0))
> > return error(_("could not configure '%s'"), dir);
> >
> > - if ((res = run_git("fetch", "--quiet", "origin", NULL))) {
> > + if ((res = run_git("fetch", "origin",
> > + verbosity ? NULL : "--quiet",
> > + NULL))) {
>
> Hmmph. This and below are a little strange in that they will end up
> calling:
>
> run_git("fetch", "origin", NULL, NULL);
>
> when running without `--verbose`. `run_git()` will still do the right
> thing and stop reading its arguments after the first NULL that it sees.
> So I doubt that it's a huge deal in practice, but felt worth calling out
> nonetheless.
>
The reason I'm doing this is seeing that toggle_maintenance() already
does this, and it's not buggy, but it's really inelegant.
My personal understanding is that the original intention of run_git()
is to help developers simply put git parameters into the variable parameters
of the function, and run_git() has no good way to understand null values.
Here we put it in run_git () The last is an act of desperation.
> Is there an opportunity to easily test this new code?
>
It's a bit cumbersome, but I will try.
> Thanks,
> Taylor
Thanks,
ZheNing Hu
