Subject: Re: [PATCH v7 4/7] submodule: refactor is_submodule_modified()

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 07 Feb 2023 23:59:05 +0100

On Tue, Feb 07 2023, Calvin Wan wrote:

> diff --git a/submodule.c b/submodule.c
> index 768d4b4cd7..d88aa2c573 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -28,6 +28,10 @@ static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
>  static int initialized_fetch_ref_tips;
>  static struct oid_array ref_tips_before_fetch;
>  static struct oid_array ref_tips_after_fetch;
> +static const char *status_porcelain_start_error =
> +	N_("could not run 'git status --porcelain=2' in submodule %s");
> +static const char *status_porcelain_fail_error =
> +	N_("'git status --porcelain=2' failed in submodule %s");

Let's instead do:

	#define STATUS_PORCELAIN_START_ERROR \
        	N_("could not run 'git status --porcelain=2' in submodule %s")
	#define STATUS_PORCELAIN_FAIL_ERROR \
        	N_("'git status --porcelain=2' failed in submodule %s")

Because a thing you're not discussing in the commit message is that the
disadvantage of doing this sort of thing is that we lose the checking
that -Wformat gives us (try to add an extra "%s" to these in your
version, then the macro version, with gcc and/or clang).

Personally I'd prefer just copy/pasting over losing that, but using a
macro instead of a variable allows us to have our cake and eat it too.