On Sat, 8 May 2010, Ævar Arnfjörð Bjarmason wrote:
> On Fri, May 7, 2010 at 20:46, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> > > Known bugs:
> > >
> > > * Breaks the model of being able to *set* config values. That
> > > doesn't work for the included files. Maybe not a bug.
> >
> > Errr... do I understand correctly that it simply means that you are
> > not able to set config values that came from included files, in
> > included files?
> >
> > This is quite serious limitation.
I was wrong there; this is not even a problem.
> It is. And recap, you can now you can set Git's config in either
> places .git/config, ~/.gitconfig and $prefix/etc/gitconfig.
>
> With inclusion this is a bit more complex. If my ~/.gitconfig includes
> a seekrt.key=foobar via an include in ~/.gitconfig/seekrt, what should
> `git config --global seekrt.key newkey` do? How about `git config
> --global seekrt.some_new_value blah`?
>
> I think it's best to not try to get into that mess and just let the
> user manage included files manually, or with `git config --file`.
This is not a problem: while "git config --get foo.bar" would search
through $GIT_DIR/config, ~/.gitconfig and /etc/gitconfig (and with
your addition also included files), "git config foo.bar baz" would set
foo.bar to baz always in per-repository config file (in absence of
--global / --system / --file=<file> option).
So this would be simply an extension of existing situation. Not a bug.
> > > * The whole bit with saving/restoring global state for config
> > > inclusion is evil, but then again so is the global state.
> >
> > Why not encapsulate those global variables in a struct, passed to
> > appropriate functions, with a global variable holding an instance of
> > such struct (IIRC similarly to what is done for "the_index").
>
> That's indeed the sane way to go. I'll do that (and look at
> the_index).
Note that variable might not be called the_index...
[...]
> > > +cat > .git/config << EOF
> > > +[some]
> > > + variable = blah
> > > +[voodoo]
> > > + include = .git/more_config_*
> > > +EOF
> >
> > I don't like this syntax.
>
> Me neither.
>
> > First, it forces git-config to hide all 'include' keys. I think
> > there might be some legitimate <section>.include config variables
> > (perhaps outside git-core); with this patch they are impossible.
Here I didn't notice that it has to be voodoo.include, and not any other
fully qualified variable name, i.e. section name must be voodoo.
> It's only hiding the full 'voodoo.include' key currently, you can
> still have e.g. 'bleh.include'.
voodoo.include shows that black magic voodoo; include.file could be
a bit better.
> > I would propose
> >
> > include .git/more_config_*
> >
> > if not for the fast that it would trip older git. Perhaps
> >
> > ## include ".git/more_config_*"
Or perhaps
#include ".git/more_config_*"
:-)
>
> Probably not a good idea to mix up comments & configuration like
> that. Some (semi-broken) parsers of .gitconfig also use INI parsers to
> parse it, which breaks on # comments. Those are already broken, but it
> would be nice if a feature didn't require them.
BTW IIRC ini-files format (at least one of them) allows for ';'-prefixed
line comments (comment must be on its own line); I wonder how it is with
ini-like git config format.
But in some ini-formats definition we have that both the hash mark (#)
and the semicolon (;) are comment characters.
>
> > [include .git/more_config_*]
>
> Syntax error on older Gits.
>
> > [include ".git/more_config_*"]
>
> I like this one the best. It's also easy to modify the parser (so it
> doesn't think it's a section) to handle it. And it doesn't incur the
> confusion of looking like a normal configuration variable.
What I don't like with this proposal is that one could write
[include ".git/more_config_*"]
foo = bar
Which is confusing.
But perhaps we can break backwards compatibility here. I don't know...
<include .git/more_config_*>
[[.git/more_config_*]]
{{.git/more_config_*}}
[=.git/more_config_*=]
[@.git/more_config_*]
%include ".git/more_config_*"
@INCLUDE = .git/more_config_*
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html