Re: [PATCH v5 0/2] Conditional config includes based on remote URL

> >>  * Inlining the "remote_urls" in the struct makes its management easier;
> >>    and the free/NULL checks just check .nr now, and string_list_clear() can be
> >>    unconditional.
> >
> > I don't think we can do this - nr might still be 0 after a scan if we
> > don't have remote URLs for some reason, so we still need to distinguish
> > between not-scanned and scanned-with-zero-URLs.
> 
> You mean so that we don't double-free? The way string_list_clear()
> protects against that, but maybe there's something else.
> 
> Whatever it is (if there's anything) it could use test coverage then :)

> >>  * It would be nice if e.g. the "includeIf.hasconfig:remote.*.url globs" test
> >>    were split up by condition, but maybe that's a hassle (would need a small helper).
> >> 
> >>    Just something that would have helped while hacking on this, i.e. now most of it
> >>    was an all-or-nothing failure & peek at the trace output
> >
> > What do you mean by condition? There seems to only be one condition
> > (whether the URL is there or not), unless you were thinking of smaller
> > subdivisions.
> 
> Maybe I'm just misunderstanding the intent here, but aren't you trying
> to guard against the case of having a ~/.gitconfig that includes
> ~/.gitconfig.d/for-this-url, and *that* file in turns changes the
> remote's "url" in its config, followed by another "include if url
> matches" condition therein?
> 
> I.e. I read (more like skimmed) the documentation & test at the end as
> forbidding that, but maybe that's OK?

> > We can decide later what the future facility will be, but I envision
> > that we will not allow included files to set config that can affect any
> > include directives in use. So, for example, if I have a user.email-based
> > include, none of my config-conditionally included files can set user.email.
> 
> I didn't look deeply at the implementation at all, but why would this be
> a problem?
> 
> You parse ~/.gitconfig, it has user.name=foo, then right after in that
> file we do:
> 
>     [includeIf "hasconfig:user.name:*foo*"]
>     path = ~/.gitconfig.d/foo
> 
> Now the top of  ~/.gitconfig.d/foo we have:
> 
>     [user]
>     name = bar
>     [includeIf "hasconfig:user.name:*bar*"]
>     path = ~/.gitconfig.d/bar
> 
> Why would it matter that we included on user.name=foo before?
> 
> Doesn't that only matter *while* we process that first "path" line? Once
> we move past it we update our configset to user.name=bar once we hit the
> "name" line of the included file.
> 
> Then when we get another "hasconfig:user.name" we just match it to our
> current user.name=*bar*.
> 
> No?
> 
> Anyway, I think it's fine to punt on it for now or whatever, just
> curious...