Re: [PATCH 11/17] pathspec: factor global magic into its own function

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

 



On Thu, Dec 8, 2016 at 5:39 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> On 12/07, Duy Nguyen wrote:
>> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
>> > Create helper functions to read the global magic environment variables
>> > in additon to factoring out the global magic gathering logic into its
>> > own function.
>> >
>> > Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
>> > ---
>> >  pathspec.c | 120 +++++++++++++++++++++++++++++++++++++------------------------
>> >  1 file changed, 74 insertions(+), 46 deletions(-)
>> >
>> > diff --git a/pathspec.c b/pathspec.c
>> > index 5afebd3..08e76f6 100644
>> > --- a/pathspec.c
>> > +++ b/pathspec.c
>> > @@ -87,6 +87,74 @@ static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
>> >         strbuf_addf(sb, ",prefix:%d)", prefixlen);
>> >  }
>> >
>> > +static inline int get_literal_global(void)
>> > +{
>> > +       static int literal_global = -1;
>> > +
>> > +       if (literal_global < 0)
>> > +               literal_global = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT,
>> > +                                             0);
>>
>> These zeros look so lonely. I know it would exceed 80 columns if we
>> put it on the previous line. But I think it's ok for occasional
>> exceptions. Or you could rename noglob_global to noglob.
>
> I was thinking the same thing but was so torn between the char limit.  I
> think it's probably ok to rename these vars by drooping the global since
> the function name themselves indicate they are global.

Exactly. I almost suggested just "ret" for that reason, but it was a
bit on the extreme side, relying entirely on the function's name for
context.
-- 
Duy



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