threads / patch / 49061

patchworktree: add --quiet option

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

## tl;dr

5 messages between Aug 7, 2018 and Aug 7, 2018. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Elia Pinto· Aug 7, 2018, 13:21 UTC · lore

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(-)
Show changes to 2 files +12 −3

Documentation/git-worktree.txt, builtin/worktree.c

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
Martin Ågren· Aug 7, 2018, 14:37 UTC · re: Elia Pinto · lore

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

Hi Elia
On 7 August 2018 at 15:21, Elia Pinto <gitter.spiros@gmail.com> wrote:
Show 18 quoted lines
> 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...
Show 7 quoted lines
> @@ -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.
Show 5 quoted lines
> @@ -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

Elia Pinto· Aug 7, 2018, 15:46 UTC · re: Martin Ågren · lore

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

2018-08-07 16:37 GMT+02:00 Martin Ågren <martin.agren@gmail.com>:
Show 66 quoted lines
> 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.
Duy Nguyen· Aug 7, 2018, 15:30 UTC · re: Elia Pinto · lore

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

On Tue, Aug 7, 2018 at 3:27 PM Elia Pinto <gitter.spiros@gmail.com> wrote:
Show 22 quoted lines
>
> 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?

Show 14 quoted lines
>                 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
Eric Sunshine· Aug 7, 2018, 19:12 UTC · re: Elia Pinto · lore

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

(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.

Show 12 quoted lines
> 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/

← back to recent threads