Re: [PATCH 3/7] submodule-config: keep labels around

From: Junio C Hamano <gitster@pobox.com>
Date: May 11, 2016, 01:15 UTC
Message-ID: <xmqqh9e5l7id.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1462928397-1708-4-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 5 quoted lines

> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>  	submodule->update_strategy.command = NULL;
>  	submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>  	submodule->ignore = NULL;
> +	submodule->labels = NULL;

Hmph, is there a reason to do this, instead of embedding an instance of "struct string_list" inside submodule structure?

I am not yet claiming that embedding is better. Just wondering if it makes it easier to handle initialization as seen in the hunk below, and also _clear() procedure.

Show 16 quoted lines

> @@ -353,6 +358,17 @@ static int parse_config(const char *var, const char *value, void *data)
>  		else if (parse_submodule_update_strategy(value,
>  			 &submodule->update_strategy) < 0)
>  				die(_("invalid value for %s"), var);
> +	} else if (!strcmp(item.buf, "label")) {
> +		if (!value)
> +			ret = config_error_nonbool(var);
> +		else {
> +			if (!submodule->labels) {
> +				submodule->labels =
> +					xmalloc(sizeof(*submodule->labels));
> +				string_list_init(submodule->labels, 1);
> +			}
> +			string_list_insert(submodule->labels, value);
> +		}
>  	}