Re: [PATCH v9 3/8] add line number and file name info to `config_set`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes:

> On 07/08/14 12:59, Tanay Abhra wrote:
>> Store file name and line number for each key-value pair in the cache
>> during parsing of the configuration files.
>> 
>> Signed-off-by: Tanay Abhra <tanayabh@xxxxxxxxx>
>> ---
>>  cache.h  |  5 +++++
>>  config.c | 16 ++++++++++++++--
>>  2 files changed, 19 insertions(+), 2 deletions(-)
>> 
>> diff --git a/cache.h b/cache.h
>> index 7292aef..0b1bdfd 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -1383,6 +1383,11 @@ extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);
>>  extern int git_config_get_maybe_bool(const char *key, int *dest);
>>  extern int git_config_get_pathname(const char *key, const char **dest);
>>  
>> +struct key_value_info {
>> +	const char *filename;
>> +	int linenr;
>> +};
>> +
>
> I haven't checked, but does this series now include a user for
> this struct outside of config.c? If not, then I think it would
> be better to leave the declaration in config.c until it is needed.
> (To make it easier to see if it is necessary in the context of the
> patch which will make use of it).

I disagree: this patch series is essentially about introducing a new
API, and this struct declaration is part of the API.

It seemed strange to me to see the code movement in the patch from two
versions of the series, but the patch itself does not move the code, it
just adds new code directly where it belongs.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]