Re: [PATCH v2] worktree: add --quiet option
From: Eric Sunshine <hidden>
Date: 2018-08-16 08:43:58
On Wed, Aug 15, 2018 at 4:56 PM Elia Pinto [off-list ref] wrote:
Add the '--quiet' option to git worktree, as for the other git commands. 'add' is the only command affected by it since all other commands, except 'list', are currently silent by default.
Nit: wrap the commit message at around 70 columns rather than 45 or so.
quoted hunk ↗ jump to hunk
Signed-off-by: Elia Pinto <redacted> ---diff --git a/builtin/worktree.c b/builtin/worktree.c@@ -303,9 +304,13 @@ static int add_worktree(const char *path, const char *refname, + else { argv_array_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL); + if (opts->quiet) + argv_array_push(&cp.args, "--quiet"); + }
This constructs the command as "git symbolic-ref HEAD <ref> --quiet".
Although that's not wrong per-se, it does perhaps set an undesirable
precedent. A more standard construction would be:
argv_array_push(..., "symbolic-ref");
if (opts->quiet)
argv_array_push(..., "--quiet");
argv_array_pushl(..., "HEAD", symref.buf, NULL);
I used "pushl" on the last one to indicate the semantic relationship
between those two arguments.
+
Nit: the above code is directly related to the code just below, so the new blank line you add here somewhat (undesirably) divorces the pieces of code from each other
quoted hunk ↗ jump to hunk
cp.env = child_env.argv; ret = run_command(&cp); if (ret)@@ -315,6 +320,8 @@ static int add_worktree(const char *path, const char *refname, argv_array_pushl(&cp.args, "reset", "--hard", NULL); + if (opts->quiet) + argv_array_push(&cp.args, "--quiet");
I could also see this one re-written as:
argv_array_push(...,"reset");
argv_array_push(...,"--hard");
if (opts->quiet)
argv_array_push(...,"--quiet");
now that the command invocation has added another option.
Not at all worth a re-roll.
quoted hunk ↗ jump to hunk
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh@@ -252,6 +252,11 @@ test_expect_success 'add -B' ' +test_expect_success 'add --quiet' ' + git worktree add --quiet ../foo master >expected 2>&1 && + test_must_be_empty expected +'
This test is going to create the new worktree directly in Git's t/ directory due to "../foo". Please don't do that. Use a name without the leading "../". Also, you should make sure that the worktree doesn't already exist (wasn't created by an earlier test), otherwise the "git worktree add" command will fail. So, either choose a distinct worktree name or use "test_might_fail git worktree remove -f <worktree>" before the "git worktree add" command.