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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 7, 2023, 22:59 UTC
Message-ID: <230208.86pmal13jo.gmgdl@evledraar.gmail.com>
In-Reply-To: <20230207181706.363453-5-calvinwan@google.com>

On Tue, Feb 07 2023, Calvin Wan wrote:

Show 12 quoted lines

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