Re: Implementing branch attributes in git config

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

 



Hi,

On Sun, 7 May 2006, Junio C Hamano wrote:

> Pavel Roskin <proski@xxxxxxx> writes:
> 
> > In particular, git-repo-config leaves the config file locked in the
> > regex is wrong:
> >
> > $ git-repo-config branch.fetch "master:origin" +
> > Invalid pattern: +
> > $ git-repo-config branch.fetch "master:origin" +
> > could not lock config file
> >
> > To fix it, just add "close(fd); unlink(lock_file);" after "Invalid
> > pattern" in config.c.
> 
> I'd give Johannes the first refusal right to deal with this and
> not touch repo-config.c myself for now, since I suspect I
> tempted him enough to restructure it ;-).

Thanks. Yes, you tempted me real hard :-) Unfortunately, the restructuring 
will have to wait a little, because I really should work for my day job 
these days... :-(

> > I don't quite understand what pattern is needed to add an entry.  "foo"
> > seems to work fine, I don't know why.
> 
> I think the value regexp is "replace the ones that match this",
> and the convention he came up with is to use "^$" to append (see
> some examples in t/t1300-repo-config.sh).
> 
> In any case, Documentation/git-repo-config.txt mentions
> value_regex without explaining what the semantics is.  This
> needs to be fixed, probably like the attached patch.
> 
> > That problem with multiple values is that they are quite fragile and
> > require special options to access them.  Since regex is used, dots in
> > the branch names need to be escaped.  Probably more escapes are needed.
> 
> I have a suspicion that using regex while is more powerful and
> expressive might be a mistake and it would be easier for users
> (both Porcelain and end-users) to use fnmatch() patterns.

I did not know about fnmatch()... It is probably better than regular 
expressions. After all, what I use it for most often is

	git-repo-config --get-all remote.*url

which -- magically -- will continue to work with fnmatch()!

> 	[branch]
> 		defaultremote = origin for master
>                 defaultremote = private for test

FWIW I like that syntax much better. But then, somebody called me weird 
because of how I order the arguments of a comparison... tsk, tsk.

> @@ -23,10 +23,11 @@ You can query/set/replace/unset options 
>  actually the section and the key separated by a dot, and the value will be
>  escaped.
>  
> -If you want to set/unset an option which can occur on multiple lines, you
> -should provide a POSIX regex for the value. If you want to handle the lines
> -*not* matching the regex, just prepend a single exclamation mark in front
> -(see EXAMPLES).
> +If you want to set/unset an option which can occur on multiple
> +lines, a POSIX regexp `value_regex` needs to be given.  Only the
> +existing values that match the regexp are updated or unset.  If
> +you want to handle the lines that do *not* match the regex, just
> +prepend a single exclamation mark in front (see EXAMPLES).

I would actually prefer to go with your suggestion of using shell patterns 
instead of regular expressions. They are not needed, and most users tend 
to positively hate regular expressions.

Thoughts?

Ciao,
Dscho

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

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