Re: [PATCH v2 2/2] fetch: add cli option --default-only

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 5, 2024, 02:43 UTC
Message-ID: <CAPig+cRo6pFnPN6PDka+jEJ2Y5O1KSgtzwdPcTpamM=-p_hkcQ@mail.gmail.com>
In-Reply-To: <20240104222259.15659-2-dev@tb6.eu>

On Thu, Jan 4, 2024 at 5:23 PM Tamino Bauknecht <dev@tb6.eu> wrote:

Show 11 quoted lines

> This option can be used to restore the default behavior of "git fetch"
> if the "fetch.all" config option is enabled.
> The flag cannot be used in combination with "--all" or explicit
> remote(s).
>
> Signed-off-by: Tamino Bauknecht <dev@tb6.eu>
> ---
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> @@ -2344,15 +2347,23 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> +       if (all && default_only) {
> +               die(_("fetch --all does not work with fetch --default-only"));

To simplify the life of people who translate Git messages into other languages, these days we have standard wording for this type of message, and we extract the literal option from the message itself. So, this should be:

    die(_("options '%s' and '%s' cannot be used together"),
        "--all", "--default-only");

Show 21 quoted lines

> diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
> @@ -304,4 +304,45 @@ test_expect_success 'git config fetch.all false (fetch only default remote)' '
> +for fetch_all in true false
> +do
> +       test_expect_success "git fetch --default-only (fetch only default remote with fetch.all = $fetch_all)" '
> +               test_dir="test_default_only_$fetch_all" &&
> +               setup_test_clone "$test_dir" &&
> +               (
> +                       cd "$test_dir" &&
> +                       git config fetch.all $fetch_all &&
> +                       git fetch --default-only &&
> +                       cat >expect <<-\EOF &&
> +                         origin/HEAD -> origin/main
> +                         origin/main
> +                         origin/side
> +                       EOF
> +                       git branch -r >actual &&
> +                       test_cmp expect actual
> +               )
> +       '
> +done

Do you also want to test the case when "fetch.all" isn't set?

Show 6 quoted lines

> +test_expect_success 'git fetch --all does not work with --default-only' '
> +       (
> +               cd test &&
> +               test_must_fail git fetch --all --default-only
> +       )
> +'

Minor point: This sort of test can be written more succinctly without the subshell:

    test_expect_success 'git fetch --all does not work with --default-only' '
        test_must_fail git -C test fetch --all --default-only
    '