Re: [PATCH v3 14/23] ref_filter: add is_atom_used function

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

 



2018-02-15 8:49 GMT+03:00 Jeff King <peff@xxxxxxxx>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Delete all items related to split_on_whitespace from ref-filter
>> and add new function for handling the logic.
>> Now cat-file could invoke that function to implementing its logic.
>
> OK, this is a good direction. I think in a more compact series we'd
> avoid moving the split-on-whitespace bits over to ref-filter in the
> first place, and have two commits:
>
>  - one early in the series adding is_atom_used()
>
>  - one late in the series switching cat-file over to is_atom_used() as
>    part of the conversion to ref-filter
>
>> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
>> index 6db57e3533806..3a49b55a1cc2e 100644
>> --- a/builtin/cat-file.c
>> +++ b/builtin/cat-file.c
>> @@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
>>  {
>>       struct strbuf buf = STRBUF_INIT;
>>       struct expand_data data;
>> -     int save_warning;
>> -     int retval = 0;
>> +     int save_warning, is_rest, retval = 0;
>
> Try to avoid reformatting existing code that you're not otherwise
> touching, as it makes the diff noisier. Just adding "int is_rest" would
> make this easier to review.
>
> I also think the variable name should probably still be
> "split_on_whitespace". It's set based on whether we saw a "%(rest)"
> atom, but ultimately we'll use it to decide whether to split.

OK, I will fix that.

>
> -Peff



[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]

  Powered by Linux