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

2016-12-08 Thread Duy Nguyen 
On Thu, Dec 8, 2016 at 5:39 AM, Brandon Williams  wrote:
> On 12/07, Duy Nguyen wrote:
>> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams  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 
>> > ---
>> >  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

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

2016-12-07 Thread Brandon Williams 
On 12/07, Duy Nguyen wrote:
> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams  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 
> > ---
> >  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.
-- 
Brandon Williams

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

2016-12-07 Thread Duy Nguyen 
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams  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 
> ---
>  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.
-- 
Duy

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

2016-12-06 Thread Brandon Williams 
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 
---
 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);
+   return literal_global;
+}
+
+static inline int get_glob_global(void)
+{
+   static int glob_global = -1;
+
+   if (glob_global < 0)
+   glob_global = git_env_bool(GIT_GLOB_PATHSPECS_ENVIRONMENT, 0);
+   return glob_global;
+}
+
+static inline int get_noglob_global(void)
+{
+   static int noglob_global = -1;
+
+   if (noglob_global < 0)
+   noglob_global = git_env_bool(GIT_NOGLOB_PATHSPECS_ENVIRONMENT,
+0);
+   return noglob_global;
+}
+
+static inline int get_icase_global(void)
+{
+   static int icase_global = -1;
+
+   if (icase_global < 0)
+   icase_global = git_env_bool(GIT_ICASE_PATHSPECS_ENVIRONMENT, 0);
+
+   return icase_global;
+}
+
+static int get_global_magic(int element_magic)
+{
+   int global_magic = 0;
+
+   if (get_literal_global())
+   global_magic |= PATHSPEC_LITERAL;
+
+   /* --glob-pathspec is overridden by :(literal) */
+   if (get_glob_global() && !(element_magic & PATHSPEC_LITERAL))
+   global_magic |= PATHSPEC_GLOB;
+
+   if (get_glob_global() && get_noglob_global())
+   die(_("global 'glob' and 'noglob' pathspec settings are 
incompatible"));
+
+   if (get_icase_global())
+   global_magic |= PATHSPEC_ICASE;
+
+   if ((global_magic & PATHSPEC_LITERAL) &&
+   (global_magic & ~PATHSPEC_LITERAL))
+   die(_("global 'literal' pathspec setting is incompatible "
+ "with all other global pathspec settings"));
+
+   /* --noglob-pathspec adds :(literal) _unless_ :(glob) is specified */
+   if (get_noglob_global() && !(element_magic & PATHSPEC_GLOB))
+   global_magic |= PATHSPEC_LITERAL;
+
+   return global_magic;
+}
+
 /*
  * Take an element of a pathspec and check for magic signatures.
  * Append the result to the prefix. Return the magic bitmap.
@@ -105,46 +173,12 @@ static unsigned prefix_pathspec(struct pathspec_item 
*item,
const char *prefix, int prefixlen,
const char *elt)
 {
-   static int literal_global = -1;
-   static int glob_global = -1;
-   static int noglob_global = -1;
-   static int icase_global = -1;
-   unsigned magic = 0, element_magic = 0, global_magic = 0;
+   unsigned magic = 0, element_magic = 0;
const char *copyfrom = elt;
char *match;
int i, pathspec_prefix = -1;
 
-   if (literal_global < 0)
-   literal_global = 
git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
-   if (literal_global)
-   global_magic |= PATHSPEC_LITERAL;
-
-   if (glob_global < 0)
-   glob_global = git_env_bool(GIT_GLOB_PATHSPECS_ENVIRONMENT, 0);
-   if (glob_global)
-   global_magic |= PATHSPEC_GLOB;
-
-   if (noglob_global < 0)
-   noglob_global = git_env_bool(GIT_NOGLOB_PATHSPECS_ENVIRONMENT, 
0);
-
-   if (glob_global && noglob_global)
-   die(_("global 'glob' and 'noglob' pathspec settings are 
incompatible"));
-
-
-   if (icase_global < 0)
-   icase_global = git_env_bool(GIT_ICASE_PATHSPECS_ENVIRONMENT, 0);
-   if (icase_global)
-   global_magic |= PATHSPEC_ICASE;
-
-   if ((global_magic & PATHSPEC_LITERAL) &&
-   (global_magic & ~PATHSPEC_LITERAL))
-   die(_("global 'literal' pathspec setting is incompatible "
- "with all other global pathspec settings"));
-
-   if (flags & PATHSPEC_LITERAL_PATH)
-   global_magic = 0;
-
-   if (elt[0] != ':' || literal_global ||
+   if (elt[0] != ':' || get_literal_global() ||
(flags & PATHSPEC_LITERAL_PATH)) {
; /* nothing to do */
} else if (elt[1] == '(') {
@@ -208,15 +242,9 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
 
magic |= element_magic;
 
-   /* --noglob-pathspec adds :(literal) _unless_ :(glob) is specified */
-   if (noglob_global && !(magic & PATHSPEC_GLOB))
-