Re: [PATCHv3 3/4] submodule: add flags to ok_to_remove_submodule

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 14, 2016, 23:48 UTC
Message-ID: <xmqqpokuvzqf.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161214224101.6211-4-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 29 quoted lines

> diff --git a/submodule.c b/submodule.c
> index 9f0b544ebe..2d13744b06 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1019,7 +1019,7 @@ int submodule_uses_gitfile(const char *path)
>  	return 1;
>  }
>  
> -int ok_to_remove_submodule(const char *path)
> +int ok_to_remove_submodule(const char *path, unsigned flags)
>  {
>  	ssize_t len;
>  	struct child_process cp = CHILD_PROCESS_INIT;
> @@ -1032,15 +1032,27 @@ int ok_to_remove_submodule(const char *path)
>  	if (!submodule_uses_gitfile(path))
>  		return 0;
>  
> -	argv_array_pushl(&cp.args, "status", "--porcelain", "-u",
> +	argv_array_pushl(&cp.args, "status", "--porcelain",
>  				   "--ignore-submodules=none", NULL);
> +
> +	if (flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED)
> +		argv_array_push(&cp.args, "-uno");
> +	else
> +		argv_array_push(&cp.args, "-uall");
> +
> +	if (!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED))
> +		argv_array_push(&cp.args, "--ignored");
> +

These "internal values to assemble command line" operations we cannot avoid. But things like this ...

Show 6 quoted lines

>  	if (start_command(&cp))
> -		die(_("could not run 'git status --porcelain -u --ignore-submodules=none' in submodule %s"), path);
> +		die(_("could not run 'git status --porcelain --ignore-submodules=none %s %s' in submodule '%s'"),
> +			(flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED) ? "-uno" : "-uall",
> +			(!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)) ? "--ignored" : "",
> +			path);

and this ...

Show 6 quoted lines

>  	if (finish_command(&cp))
> -		die(_("'git status --porcelain -u --ignore-submodules=none' failed in submodule %s"), path);
> +		die(_("'git status --porcelain --ignore-submodules=none %s %s' failed in submodule '%s'"),
> +			(flags & SUBMODULE_REMOVAL_IGNORE_UNTRACKED) ? "-uno" : "-uall",
> +			(!(flags & SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED)) ? "--ignored" : "",
> +			path);

makes me wonder if we want a helper that builds the string out of an already assembled cp.args[] array, so that we won't have to do the same thing twice/thrice and more importantly we won't have to worry about these three going out of sync.