[PATCH] worktree: add --quiet option

[PATCH] worktree: add --quiet option

[Elia Pinto]

Add the '--quiet' option to git worktree add,
as for the other git commands.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 Documentation/git-worktree.txt |  4 +++-
 builtin/worktree.c             | 11 +++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 9c26be40f..508cde55c 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -115,7 +115,9 @@ Unlock a working tree, allowing it to be pruned, moved or deleted.
 
 OPTIONS
 -------
-
+-q::
+--quiet::
+	With 'add', suppress feedback messages.
 -f::
 --force::
 	By default, `add` refuses to create a new working tree when
diff --git a/builtin/worktree.c b/builtin/worktree.c
index a763dbdcc..c47feb4a4 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -27,6 +27,7 @@ static const char * const worktree_usage[] = {
 struct add_opts {
 	int force;
 	int detach;
+	int quiet;
 	int checkout;
 	int keep_locked;
 };
@@ -315,6 +316,9 @@ static int add_worktree(const char *path, const char *refname,
 		cp.argv = NULL;
 		argv_array_clear(&cp.args);
 		argv_array_pushl(&cp.args, "reset", "--hard", NULL);
+		if (opts->quiet)
+			argv_array_push(&cp.args, "--quiet");
+		printf("%s\n","soo qia");
 		cp.env = child_env.argv;
 		ret = run_command(&cp);
 		if (ret)
@@ -437,6 +441,7 @@ static int add(int ac, const char **av, const char *prefix)
 		OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
+		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
 		OPT_PASSTHRU(0, "track", &opt_track, NULL,
 			     N_("set up tracking mode (see git-branch(1))"),
 			     PARSE_OPT_NOARG | PARSE_OPT_OPTARG),
@@ -491,8 +496,8 @@ static int add(int ac, const char **av, const char *prefix)
 			}
 		}
 	}
-
-	print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
+	if (!opts.quiet)
+		print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
 
 	if (new_branch) {
 		struct child_process cp = CHILD_PROCESS_INIT;
@@ -500,6 +505,8 @@ static int add(int ac, const char **av, const char *prefix)
 		argv_array_push(&cp.args, "branch");
 		if (new_branch_force)
 			argv_array_push(&cp.args, "--force");
+		if (opts.quiet)
+			argv_array_push(&cp.args, "--quiet");
 		argv_array_push(&cp.args, new_branch);
 		argv_array_push(&cp.args, branch);
 		if (opt_track)
-- 
2.18.0.721.gc1f18ed

Re: [PATCH] worktree: add --quiet option

[Martin Ågren]

Hi Elia

On 7 August 2018 at 15:21, Elia Pinto <gitter.spiros@gmail.com> wrote:
> Add the '--quiet' option to git worktree add,
> as for the other git commands.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>  Documentation/git-worktree.txt |  4 +++-
>  builtin/worktree.c             | 11 +++++++++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 9c26be40f..508cde55c 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -115,7 +115,9 @@ Unlock a working tree, allowing it to be pruned, moved or deleted.
>
>  OPTIONS
>  -------
> -

Grepping through Documentation/, it is clear that we sometimes have a
blank line here, sometimes not. I'm not sure what to make of that.

> +-q::
> +--quiet::
> +       With 'add', suppress feedback messages.
>  -f::

But I do think that for consistency, we'd prefer a blank line before `-f::`.

Both the commit message and this documentation makes me wonder if this
focuses on "add" because it's the only subcommand where `--quiet` makes
sense, conceptually, or because this is where you happen to need it
personally, or due to some other $reason. Could you say something more
about this?

I'm not a worktree power-user, so please forgive my ignorance...

> @@ -315,6 +316,9 @@ static int add_worktree(const char *path, const char *refname,
>                 cp.argv = NULL;
>                 argv_array_clear(&cp.args);
>                 argv_array_pushl(&cp.args, "reset", "--hard", NULL);
> +               if (opts->quiet)
> +                       argv_array_push(&cp.args, "--quiet");
> +               printf("%s\n","soo qia");

This last line looks like debug cruft.

> @@ -437,6 +441,7 @@ static int add(int ac, const char **av, const char *prefix)
>                 OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
>                 OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
>                 OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
> +               OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),

This matches other users. Good.

I did some simple testing and this appears to be quite quiet, modulo
the "soo qia" that I already mentioned. Could you add a test to
demonstrate the quietness and to keep it from regressing? Something like
`git worktree add ../foo >out && test_must_be_empty out" in e.g.,
t2025-worktree-add.sh might do the trick (capture stderr as well?).

Hope this helps
Martin

Re: [PATCH] worktree: add --quiet option

[Elia Pinto]

2018-08-07 16:37 GMT+02:00 Martin Ågren <martin.agren@gmail.com>:
> Hi Elia
>
> On 7 August 2018 at 15:21, Elia Pinto <gitter.spiros@gmail.com> wrote:
>> Add the '--quiet' option to git worktree add,
>> as for the other git commands.
>>
>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>> ---
>>  Documentation/git-worktree.txt |  4 +++-
>>  builtin/worktree.c             | 11 +++++++++--
>>  2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
>> index 9c26be40f..508cde55c 100644
>> --- a/Documentation/git-worktree.txt
>> +++ b/Documentation/git-worktree.txt
>> @@ -115,7 +115,9 @@ Unlock a working tree, allowing it to be pruned, moved or deleted.
>>
>>  OPTIONS
>>  -------
>> -
>
> Grepping through Documentation/, it is clear that we sometimes have a
> blank line here, sometimes not. I'm not sure what to make of that.
>
>> +-q::
>> +--quiet::
>> +       With 'add', suppress feedback messages.
>>  -f::
>
> But I do think that for consistency, we'd prefer a blank line before `-f::`.
>
> Both the commit message and this documentation makes me wonder if this
> focuses on "add" because it's the only subcommand where `--quiet` makes
> sense, conceptually, or because this is where you happen to need it
> personally, or due to some other $reason. Could you say something more
> about this?
>
> I'm not a worktree power-user, so please forgive my ignorance...
>
>> @@ -315,6 +316,9 @@ static int add_worktree(const char *path, const char *refname,
>>                 cp.argv = NULL;
>>                 argv_array_clear(&cp.args);
>>                 argv_array_pushl(&cp.args, "reset", "--hard", NULL);
>> +               if (opts->quiet)
>> +                       argv_array_push(&cp.args, "--quiet");
>> +               printf("%s\n","soo qia");
>
> This last line looks like debug cruft.
>
>> @@ -437,6 +441,7 @@ static int add(int ac, const char **av, const char *prefix)
>>                 OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
>>                 OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
>>                 OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
>> +               OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
>
> This matches other users. Good.
>
> I did some simple testing and this appears to be quite quiet, modulo
> the "soo qia" that I already mentioned. Could you add a test to
> demonstrate the quietness and to keep it from regressing? Something like
> `git worktree add ../foo >out && test_must_be_empty out" in e.g.,
> t2025-worktree-add.sh might do the trick (capture stderr as well?).
>
> Hope this helps
> Martin


Thank you all. sorry for the trash in the patch I will resend it.

Re: [PATCH] worktree: add --quiet option

[Duy Nguyen]

On Tue, Aug 7, 2018 at 3:27 PM Elia Pinto <gitter.spiros@gmail.com> wrote:
>
> Add the '--quiet' option to git worktree add,
> as for the other git commands.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>  Documentation/git-worktree.txt |  4 +++-
>  builtin/worktree.c             | 11 +++++++++--
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 9c26be40f..508cde55c 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -115,7 +115,9 @@ Unlock a working tree, allowing it to be pruned, moved or deleted.
>
>  OPTIONS
>  -------
> -
> +-q::
> +--quiet::
> +       With 'add', suppress feedback messages.

Should we update the synopsis as well?

> @@ -315,6 +316,9 @@ static int add_worktree(const char *path, const char *refname,

Before here we run either update-ref or symbolic-ref. update-ref does
not have --quiet so it's fine, no need to add another option there
(until it shows something when used with "worktree add --quiet") but
symbolic-ref seems to support -q. Should we pass -q to it?

>                 cp.argv = NULL;
>                 argv_array_clear(&cp.args);
>                 argv_array_pushl(&cp.args, "reset", "--hard", NULL);
> +               if (opts->quiet)
> +                       argv_array_push(&cp.args, "--quiet");
> +               printf("%s\n","soo qia");
>                 cp.env = child_env.argv;
>                 ret = run_command(&cp);
>                 if (ret)
> @@ -437,6 +441,7 @@ static int add(int ac, const char **av, const char *prefix)
>                 OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")),
>                 OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
>                 OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")),
> +               OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),

git grep OPT__QUIET shows that we have plenty different messages to
describe --quiet. But yeah "support progress reporting" seems close
enough in this context.

>                 OPT_PASSTHRU(0, "track", &opt_track, NULL,
>                              N_("set up tracking mode (see git-branch(1))"),
>                              PARSE_OPT_NOARG | PARSE_OPT_OPTARG),

The rest looks good.
-- 
Duy

Re: [PATCH] worktree: add --quiet option

[Eric Sunshine]

(cc:+Karen Arutyunov[1]; perhaps also do so when you re-roll)

In addition to the good review comments by Martin and Duy...

On Tue, Aug 7, 2018 at 9:21 AM Elia Pinto <gitter.spiros@gmail.com> wrote:
> Add the '--quiet' option to git worktree add,
> as for the other git commands.

It might make sense to say instead that this is adding a --quiet
option _in general_, rather than doing so specifically for 'add'.
Then, go on to say that, at present, 'add' is the only command
affected by it since all other commands are currently silent by
default (except, of course, 'list').

Whether you like that idea or not, as Martin suggested, please do say
something in the commit message about why the other commands don't
need it.

> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -115,7 +115,9 @@ Unlock a working tree, allowing it to be pruned, moved or deleted.
>  OPTIONS
>  -------
> -
> +-q::
> +--quiet::
> +       With 'add', suppress feedback messages.
>  -f::
>  --force::

In addition to the blank-line issues Martin raised, please move this
hunk downward to be a neighbor of the --verbose option.

REFERENCES
[1]: https://public-inbox.org/git/02659385-334f-2b77-c9a8-ffea8e461b0b@codesynthesis.com/