Re: [PATCH v7 3/3] config: add conditional include

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

 



On Wed, Mar 01, 2017 at 09:47:40AM -0800, Junio C Hamano wrote:

> > @@ -185,6 +271,12 @@ int git_config_include(const char *var, const char *value, void *data)
> >  
> >  	if (!strcmp(var, "include.path"))
> >  		ret = handle_path_include(value, inc);
> > +
> > +	if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
> > +	    (cond && include_condition_is_true(cond, cond_len)) &&
> > +	    !strcmp(key, "path"))
> > +		ret = handle_path_include(value, inc);
> > +
> >  	return ret;
> >  }
> 
> So "includeif.path" (misspelled one without any condition) falls
> through to "return ret" and gives the value we got from inc->fn().
> I am OK with that (i.e. "missing condition is false").

Yeah, I think that is sensible, just as not-understood nonsense like
"include.foobar" would be ignored, as well.

> Or we can make it go to handle_path_include(), effectively making
> the "include.path" a short-hand for "includeIf.path".  I am also OK
> with that (i.e. "missing condition is true").

I think we want "missing condition is false". Certainly an empty
condition like "includeIf..path" is false, as are conditions we don't
understand.

> Or we could even have "include.[<condition>.]path" without
> "includeIf"?  I am not sure if it is a bad idea that paints
> ourselves in a corner, but somehow I find it tempting.

That was how I had originally envisioned the namespace working when I
introduced include.path long ago. And I think Duy's v1 used that, but
the feedback was that it was not sufficiently obvious that the
subsection was a conditional.

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