Simon Ruderich <simon@xxxxxxxxxxxx> writes:
> On Thu, Jan 07, 2021 at 07:36:52AM +0100, Patrick Steinhardt wrote:
>> [snip]
>>
>> +void git_config_push_env(const char *spec)
>> +{
>> + struct strbuf buf = STRBUF_INIT;
>> + const char *env_name;
>> + const char *env_value;
>> +
>> + env_name = strrchr(spec, '=');
>> + if (!env_name)
>> + die("invalid config format: %s", spec);
>> + env_name++;
>> +
>> + env_value = getenv(env_name);
>> + if (!env_value)
>> + die("config variable missing for '%s'", env_name);
>
> I think "environment variable" should be mentioned in the error
> message to make it clear what kind of "variable" is missing.
Good observation. This parses foo=bar and complains about bar
missing in the environment; It is not a "config variable" that is
missing.
It is "'bar', which is supposed to be there whose value is going to
be used as the value of configuration variable 'foo', is missing."
I wonder if we should also talk about 'foo' at the same time as a
hint for what went wrong? E.g.
die(_("missing environment variable '%s' for configuration '%.*s'"),
env_name, (int)((env_name-1) - spec), spec);
I don't offhand know if that is too much info that may not be all
that useful, though.
> Btw. shouldn't these strings get translated (or does die() do
> that automatically)?
The format string given to die/error/warn should be marked with _().
Thanks.
