* Re: Implementing branch attributes in git config
From: Junio C Hamano @ 2006-05-09 18:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0605091321350.7652@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Okay, to summarize what people proposed (and that I remember):
>
> 1) [branch."AnY+String"]
>
> 2) multiple [branch]
>
> 3) magic <space>+<quoted>
>
> 4) [branch.just/allow-slashes/and-dashes]
>
> 5) the " for " notation
Thanks for a nice summary.
> Now, for the ease of use:
>
> (1), (3) and (4) are in the same league of easiness (except maybe that you
> have to keep in mind to extra-quote in shell scripts with (1) and (3)),
> (2) is especially good for people with a database mindset, and (5) is
> annoying as hell.
One thing you might want to consider is variable types and
default values (eh, that makes two).
When Linus first introduced the config mechanism, he made it so
that a loosely coupled set of programs can take the "Why should
I care about other programs configuration" attitude, and
actively encouraged to do so by allowing custom config parsers
inherit from the default parser, like the way git_diff_config()
falls back on git_default_config() when it does not recognize
the variable.
It is quite a good design for most uses, except that it made it
inconvenient to implement things like "git-repo-config -l" and
"git-var -l". The point of this design _is_ that they cannot
know what the set of possible variables, their types and the
default values when missing are, so by design the script that
used "git-var -l | grep" to read the configuration needed to
know that a boolean can be denoted by existence, value set to
zero or non-zero integer, or string "true"/"false" (i.e.
"filemode", "filemode=1", and "filemode = true" mean the same
thing; BTW I think we would probably want to add "yes"/"no"
here).
Later it was made easier to use by Pasky with "repo-config --type"
option. The caller supplies the name of the variable and the
type and repo-config gets the value -- the caller knows what it
wants to use, so having it to know what type of things it is
interested in is not so bad, so the type problem was practically
solved. But it still feels somewhat hacky.
With (2) and (5), we have a bound set of "se.ct.ion.variable";
we could enumerate the variables we care about, define what they
mean and what their types and default values are (we need to do
that for Documentation/config.txt anyway). With many parts of
the standalone git plumbing programs migrating into builtins, I
think it is not a bad idea to have a central table of all the
configuration variables that the core knows about. Porcelains
and scripts could define customized tables that describe the
sections/variables they also want to see and act on in the
configuration file, and call git-repo-config with that table as
an optional parameter.
> Now, for the ease of implementation:
>
> (1) and (3) are in the same league, they have to change the way the config
> is parsed as well as make downcasing conditional in repo-config. (2) is
> obviously hardest of all. (4) is very easy (one line in config.c), and (5)
> easiest (nothing to do).
>
> Now, for the versatility, i.e. what you can express with the syntax:
>...
> Obviously, I deem (4) the best solution ATM, because it has all the
> expressability needed, while being the simplest.
If we are shooting for "let's not do this again", I do not think
(4) without some quoting convention is good enough. Today, we
are talking about branch names so we could give them artificial
limits, which could be weaker than what we already have on the
branch names, but we would later regret that, when we start
wanting to have other names in the configuration (e.g. people's
names).
^ permalink raw reply
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Eric Wong @ 2006-05-09 19:18 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: git
In-Reply-To: <20060509120809.4d9494b9.tihirvon@gmail.com>
Timo Hirvonen <tihirvon@gmail.com> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
>
> > * unbundling of short options: -uC20n20z => -u -C20 -n20 -z
>
> Does anyone ever use this? I think this makes sense only for flags that
> don't have parameters but that would create an ugly special case. Is it
> too hard to type "-u -C=20 -n=20 -z"?
It is more for me. Many programs that I use already accept bundled
switches, and '=' and '-' are relatively far away and requires me
to stretch my hand uncomfortably (I have very small hands, and have
limited mobility in several fingers).
> > * optional argument handling (-C<num>, -M<num>)
> > -C <num> (with a space between them) has not changed,
> > however, <num> can be a sha1, or a path
>
> IMO optional arguments are usually bad idea.
>
> -C 2 (is "2" argument?)
> -C2 (-C=2 or -C -2?)
>
> Better to make it obvious there's an argument
>
> -C=2
>
> or not support optional arguments at all and "-C 2" becomes unambiguous.
git has always supported optional argument handling like this.
I'm striving for backwards compatibility with existing usage. That
means as a diff option, -C alone works, as does -C20. I've made -C 20
_not_ work because it breaks existing usage (where 20 could be a
filename, or a tree-ish). -C=20 would mean something
else, since I wanted to make pickaxe work exactly as it did before:
-S=var would search for '=var', not 'var'.
--
Eric Wong
^ permalink raw reply
* Re: Implementing branch attributes in git config
From: Linus Torvalds @ 2006-05-09 19:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vzmhr3wje.fsf@assigned-by-dhcp.cox.net>
On Tue, 9 May 2006, Junio C Hamano wrote:
>
> If we are shooting for "let's not do this again", I do not think
> (4) without some quoting convention is good enough. Today, we
> are talking about branch names so we could give them artificial
> limits, which could be weaker than what we already have on the
> branch names, but we would later regret that, when we start
> wanting to have other names in the configuration (e.g. people's
> names).
Here's my suggestion as a patch.
NOTE! This patch could be applied right now, and to all the branches (to
make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing
actually uses it.
It just makes the syntax be
[section<space>+"<randomstring>"]
where the only rule for "randomstring" is that it can't contain a newline,
and if you really want to insert a double-quote, you do it with \".
It turns that into the section name "secion.randomstring".
So you could use this for things like
[email "torvalds@osdl.org"]
name = Linus Torvalds
if you wanted to do the "email->name" conversion as part of the config
file format (I'm not claiming that is sensible, I'm just giving it as an
insane example). That would show up as the association
email.torvalds@osdl.org.name -> Linus Torvalds
which is easy to parse (the "." in the email _looks_ ambiguous, but it
isn't: you know that there will always be a single key-name, so you find
the key name with "strrchr(name, '.')" and things are entirely
unambiguous).
Now, it doesn't do any repo-config stuff, since the whole point of this is
to make this a legal syntax, not actually start _using_ it yet. If people
think this is an acceptable syntax, then pls apply to everything, and
worry about usage later.
Linus
---
diff --git a/config.c b/config.c
index 0f518c9..f3b74e0 100644
--- a/config.c
+++ b/config.c
@@ -134,6 +134,41 @@ static int get_value(config_fn_t fn, cha
return fn(name, value);
}
+static int get_extended_base_var(char *name, int baselen, int c)
+{
+ do {
+ if (c == '\n')
+ return -1;
+ c = get_next_char();
+ } while (isspace(c));
+
+ /* We require the format to be '[base "extension"]' */
+ if (c != '"')
+ return -1;
+ name[baselen++] = '.';
+
+ for (;;) {
+ int c = get_next_char();
+ if (c == '\n')
+ return -1;
+ if (c == '"')
+ break;
+ if (c == '\\') {
+ c = get_next_char();
+ if (c == '\n')
+ return -1;
+ }
+ name[baselen++] = c;
+ if (baselen > MAXNAME / 2)
+ return -1;
+ }
+
+ /* Final ']' */
+ if (get_next_char() != ']')
+ return -1;
+ return baselen;
+}
+
static int get_base_var(char *name)
{
int baselen = 0;
@@ -144,6 +179,8 @@ static int get_base_var(char *name)
return -1;
if (c == ']')
return baselen;
+ if (isspace(c))
+ return get_extended_base_var(name, baselen, c);
if (!isalnum(c) && c != '.')
return -1;
if (baselen > MAXNAME / 2)
^ permalink raw reply related
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Eric Wong @ 2006-05-09 19:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Timo Hirvonen, git
In-Reply-To: <7v8xpb73sq.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Timo Hirvonen <tihirvon@gmail.com> writes:
>
> > Eric Wong <normalperson@yhbt.net> wrote:
> >
> >> * unbundling of short options: -uC20n20z => -u -C20 -n20 -z
> >
> > Does anyone ever use this? I think this makes sense only for flags that
> > don't have parameters but that would create an ugly special case. Is it
> > too hard to type "-u -C=20 -n=20 -z"?
>
> I can already hear in my head that people would start talking
> about "git understands insane abbeviations of options". It
> might be unambiguous, but that does not change it is a bit on
> the insane side. People would probably expect -nuz can be split
> into -n -u -z, and the current handcrafted mess (although it is
> more obvious and easy to work with when reading and maintaining
> the existing code) is not abbreviation friendly, which we would
> want to do something about. But I think squashing options with
> parameters together is going a bit too far.
I think numeric parameters are unambiguous when bundled.
I'm used to things like `diff -ru10p` working, *shrug*
Non-numeric parameters can only be used if the option is at the end of the
bundled string:
git commit -sam'this is my commit message' would work
git commit -m'say hello' would also work
but git commit -mas'this is my commit message' would not work as intended
(where user wanted -a -s, too)
> > --with-r => --patch-with-raw works great
>
> I personally think this also is too much.
There are (currently) two types of abbreviations, one is the prefix one used
commonly in shell scripts: -e|--e|--ed|--edi|--edit. I think this should
always be supported as most of our shell scripts already do.
The other one tokenizes on '-' first and looks for a prefix match after
each '-'. I'd like to make that at least optional:
diff --git a/gitopt.c b/gitopt.c
index 056e163..9ca6025 100644
--- a/gitopt.c
+++ b/gitopt.c
gitopt.c
@@ -427,7 +427,7 @@ static void fallback_long(const struct o
}
/* ok, try harder, based on tokenization on '-' */
- if (found < 0) {
+ if (found < 0 && getenv("GIT_ABBREV_HARDER")) {
for (i = 0; ost[i].l || ost[i].s; i++) {
s = &(ost[i]);
if (s->l && opt_token_match(s,cur)) {
--
Eric Wong
^ permalink raw reply
* Re: [PATCH/RFC] gitopt - command-line parsing enhancements
From: Eric Wong @ 2006-05-09 19:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzmhr7fys.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
>
> > Here's my take at a new command-line option parser to reduce wear on my
> > fingers. It handles both long and short options, permuting, automatic
> > abbreviations, required arguments, optional arguments, and bundling.
>
> Taken a superficial look at it.
>
> Sounds nice, might be a tad too ambitious though. Looks
> intrusive at places.
I wasn't overly happy with the addition of global variables to existing
files and the way they're set (setup_revisions). But at least they're
static. Of course, I'm not yet certain that I haven't introduced new
bugs. All the tests pass, at least...
> And scary, especially the "eat" macros are very scary.
They look weird at first, but I think they help readability and
maintainability once you get used to them. They let you focus on the
important part of the function while hiding the boring parts from you.
Quite elegant, imho.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Timo Hirvonen @ 2006-05-09 20:10 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20060509191803.GA3676@localdomain>
Eric Wong <normalperson@yhbt.net> wrote:
> I'm striving for backwards compatibility with existing usage. That
> means as a diff option, -C alone works, as does -C20. I've made -C 20
> _not_ work because it breaks existing usage (where 20 could be a
> filename, or a tree-ish). -C=20 would mean something
> else,
I think optional arguments are still confusing. We could support both
-C (no args) and -C=20 syntax.
> since I wanted to make pickaxe work exactly as it did before:
> -S=var would search for '=var', not 'var'.
Some other options use -x=y syntax so this would be confusing. Pickaxe's
-Stext syntax is a bit strange. I think -S text or -S=text would be
more logical.
--
http://onion.dynserv.net/~timo/
^ permalink raw reply
* Re: Implementing branch attributes in git config
From: sean @ 2006-05-09 19:44 UTC (permalink / raw)
To: Linus Torvalds; +Cc: junkio, Johannes.Schindelin, git
In-Reply-To: <Pine.LNX.4.64.0605091215340.3718@g5.osdl.org>
On Tue, 9 May 2006 12:24:02 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:
> NOTE! This patch could be applied right now, and to all the branches (to
> make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing
> actually uses it.
>
> It just makes the syntax be
>
> [section<space>+"<randomstring>"]
>
> where the only rule for "randomstring" is that it can't contain a newline,
> and if you really want to insert a double-quote, you do it with \".
Lightly tested here. Looks good.
Sean
^ permalink raw reply
* Re: [PATCH/RFC] gitopt - command-line parsing enhancements
From: Junio C Hamano @ 2006-05-09 20:28 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20060509194825.GC3676@localdomain>
Eric Wong <normalperson@yhbt.net> writes:
>> And scary, especially the "eat" macros are very scary.
>
> They look weird at first, but I think they help readability and
> maintainability once you get used to them. They let you focus on the
> important part of the function while hiding the boring parts from you.
> Quite elegant, imho.
Sorry, there is no elegance to it as far as I can see. A macro
invocation that creates a private function while it does not
look like a function definition is already bad, you cannot have
a comma in the stmt part, and the bare semicolons in the
parenthesised text look insane.
If your patch were like this, it would have been a bit easier
for me to understand what was going on during my first pass:
static struct exec_args *ui_optparse
(struct opt_spec *s, int ac, char **av, int *ac_p, int what)
{
struct exec_args *ea = one_arg(s, ac, av, ac_p);
if (!ea) return NULL;
switch (what) {
case IGNORE_MISSING:
not_new = 1; break;
case VERBOSE:
verbose = 1; break;
case HELP:
usage(update_index_usage); break;
}
return nil_exec_args(ea);
}
instead of
gitopt_eat(opt_ignore_missing, not_new = 1;)
gitopt_eat(opt_verbose, verbose = 1;)
gitopt_eat(opt_h, usage(update_index_usage);)
Then, you would give an extra element in the table, and your
argument parsing/splitting routine passes that one to the
handler function:
static const struct opt_spec update_index_ost[] = {
...
{ "ignore-missing", 0, 0, 0, ui_optparse, IGNORE_MISSING },
{ "verbose", 0, 0, 0, ui_optparse, VERBOSE },
{ "help", 'h', 0, 0, ui_optparse, HELP },
{ 0, 0 }
Another thing is I do not think we would want to make the
argument parsing into callback style interface like you did. It
actively encourages the option variables to be global (you could
make it file scoped static but they are global nevertheless).
If you can make it an iterator style, it would be a lot easier
to see what is going on, I suspect. Then you would not even
need the callback function pointers and small functions created
by magic eat() macros.
^ permalink raw reply
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Junio C Hamano @ 2006-05-09 20:35 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: git, Eric Wong
In-Reply-To: <20060509231031.b62576da.tihirvon@gmail.com>
Timo Hirvonen <tihirvon@gmail.com> writes:
> I think optional arguments are still confusing. We could support both
> -C (no args) and -C=20 syntax.
Actually, optional numeric arguments are the norm not exception.
Think of "diff -u" vs "diff -u20" for example. Also I think it
is conventional not to use = for single-letter single-dash
options, so -C (no args -- use the default number of the
implementation whatever it is) and -C20 (the same behaviour in
principle as -C, but use my number instead of the default) are
sane, while -C=20 _is_ odd.
Having said that, I think abbreviating -u20 -n -r to -u20nr
going too far (-nru20 would be palatable perhaps), even if the
implementation allows such, unless you are entering "obfusucated
command line parameters" contest.
^ permalink raw reply
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Timo Hirvonen @ 2006-05-09 21:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, normalperson
In-Reply-To: <7vlktb2ayy.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Timo Hirvonen <tihirvon@gmail.com> writes:
>
> > I think optional arguments are still confusing. We could support both
> > -C (no args) and -C=20 syntax.
>
> Actually, optional numeric arguments are the norm not exception.
> Think of "diff -u" vs "diff -u20" for example. Also I think it
> is conventional not to use = for single-letter single-dash
> options, so -C (no args -- use the default number of the
> implementation whatever it is) and -C20 (the same behaviour in
> principle as -C, but use my number instead of the default) are
> sane, while -C=20 _is_ odd.
OK, if we don't support bundling flags at all then -x=y and -xy would do
the same thing and pickaxe's -Stext would work too. But we could not
make option flag parsing global then (-S=value -> search "=value" or
"value"?).
Maybe we should just change -Stext to -S=text or -S text.
Better to support only -x=y or -x y, not both.
> Having said that, I think abbreviating -u20 -n -r to -u20nr
> going too far
Yes
> (-nru20 would be palatable perhaps),
No :)
--
http://onion.dynserv.net/~timo/
^ permalink raw reply
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Junio C Hamano @ 2006-05-09 21:11 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: git
In-Reply-To: <20060510000826.1a708c03.tihirvon@gmail.com>
Timo Hirvonen <tihirvon@gmail.com> writes:
> Better to support only -x=y or -x y, not both.
Didn't I just say -x=y where x is a single letter _is_ odd?
It is either -xy or -x y, not -x=y.
^ permalink raw reply
* Re: [PATCH/RFC] gitopt - command-line parsing enhancements
From: Eric Wong @ 2006-05-09 21:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr7332ba0.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> If you can make it an iterator style, it would be a lot easier
> to see what is going on, I suspect. Then you would not even
> need the callback function pointers and small functions created
> by magic eat() macros.
That sounds like a great idea, I'll work on it tonight.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH 1/6] gitopt: a new command-line option parser for git
From: Timo Hirvonen @ 2006-05-09 21:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7j4u3nv8.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Timo Hirvonen <tihirvon@gmail.com> writes:
>
> > Better to support only -x=y or -x y, not both.
>
> Didn't I just say -x=y where x is a single letter _is_ odd?
> It is either -xy or -x y, not -x=y.
Oh, I thought parameters would use same syntax for short and long
options. For optional args -C2 would make sense but -C 2 would be
ambiguous ("-C -- 2" or "-C2"?). Maybe I'm just too drunk to
understand.
--
http://onion.dynserv.net/~timo/
^ permalink raw reply
* [BUG] "cg-object-id -p" ignore grafts, breaks cg-admin-rewritehist
From: Yann Dirson @ 2006-05-09 22:25 UTC (permalink / raw)
To: GIT list; +Cc: Petr Baudis
Currently (cogito 0.17.2), using "cg-object-id -p" to lookup a
commit's parents fetches information directly from the commit object
through "git-cat-file commit". This causes all its callers to ignore
any grafts, and probably causes various problems. The one I stumbled
upon is an inconstency in the data seen by cg-admin-rewritehist, when
a graft is used to replace the single parent of a commit with another
single parent - tentative recovery of a tarball import done on a wrong
branch, in the hope that cg-admin-rewritehist would allow to fix the
history as defined by the graft.
In that case, after identifying the commits to rewrite through legal
means, rewritehist attempts to lookup the parents for each of those
original commits and map them to already-rewritten ones, but
cg-object-id returns the pre-graft parent, which was not to be
rewritten, and the tool fails (miserably with an invalid rewritten
branch, as the exception is not caught).
A patch follows (depending on an updated "set -e" patch for
rewritehist) to have rewritehist at least abort in error when it can
identify such an inconsistency.
What should cg-object-id use to lookup parent information in a sane
way that does not ignore grafts ?
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
http://ydirson.free.fr/ | Check <http://www.debian.org/>
^ permalink raw reply
* Re: Implementing branch attributes in git config
From: sean @ 2006-05-09 22:09 UTC (permalink / raw)
To: torvalds; +Cc: junkio, Johannes.Schindelin, git
In-Reply-To: <BAYC1-PASMTP02C02EAC2F64AC00BB5801AEA90@CEZ.ICE>
On Tue, 9 May 2006 15:44:59 -0400
sean <seanlkml@sympatico.ca> wrote:
> On Tue, 9 May 2006 12:24:02 -0700 (PDT)
> Linus Torvalds <torvalds@osdl.org> wrote:
>
> > NOTE! This patch could be applied right now, and to all the branches (to
> > make 1.x, 1.2.x and 1.3.x all support the _syntax_). Even if nothing
> > actually uses it.
> >
> > It just makes the syntax be
> >
> > [section<space>+"<randomstring>"]
> >
> > where the only rule for "randomstring" is that it can't contain a newline,
> > and if you really want to insert a double-quote, you do it with \".
>
> Lightly tested here. Looks good.
>
Linus,
I really tried to like your patch ;o) But it breaks the repo-config command
line and causes mixing of some branches using old style [branch.xyz] and new
style [branch "XYZ"] which just doesn't seem to be the right thing to do.
The following patch produced for sake of discussion just allows section
headers to contain whatever characters are needed to get the job done.
git-repo-config works properly with no further need of change. Section
headers become case sensitive but key identifiers within sections do not.
AFAIK there should be no backward compatibility issues with regard to
case sensitivity. All tests pass after having been fixed up to remove
the assumption that section names are case insensitive.
The syntax is:
[<random string>]
Here's how your example would look in this style:
[email.torvalds@osdl.org]
name = Linus Torvalds
And there's no strange syntax needed with repo-config to set and get it:
$ git repo-config email.torvalds@osdl.org.name
Linus Torvalds
and just to show that key names are still case insensitive:
$ git repo-config email.torvalds@osdl.org.NAME
Linus Torvalds
Setting new sections is unambiguous from the command line and
doesn't have to decide whether to use the [branch "<string>"] or
[branch.section.name] format.
$ git repo-config branch.branch.x y
$ git repo-config branch.WonkKY.x y
$ git repo-config --get-regexp branch.\*
branch.branch.x y
branch.WonkKY.x y
[email.torvalds@osdl.org]
name = Linus Torvalds
[branch.branch]
x = y
[branch.WonkKY]
x = y
Sean
---
config.c | 11 +++++++----
repo-config.c | 8 ++++----
t/t1300-repo-config.sh | 38 ++++++++++++++++++++++----------------
3 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/config.c b/config.c
index 0f518c9..5d19ae9 100644
--- a/config.c
+++ b/config.c
@@ -144,11 +144,14 @@ static int get_base_var(char *name)
return -1;
if (c == ']')
return baselen;
- if (!isalnum(c) && c != '.')
- return -1;
+ if (c == '\\') {
+ c = get_next_char();
+ if (c == '\n')
+ return -1;
+ }
if (baselen > MAXNAME / 2)
return -1;
- name[baselen++] = tolower(c);
+ name[baselen++] = c;
}
}
@@ -455,7 +458,7 @@ int git_config_set_multivar(const char*
ret = 1;
goto out_free;
} else
- store.key[i] = tolower(key[i]);
+ store.key[i] = key[i];
store.key[i] = 0;
/*
diff --git a/repo-config.c b/repo-config.c
index 63eda1b..ba5fbd6 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -65,11 +65,11 @@ static int show_config(const char* key_,
static int get_value(const char* key_, const char* regex_)
{
int i;
+ char *tl;
- key = malloc(strlen(key_)+1);
- for (i = 0; key_[i]; i++)
- key[i] = tolower(key_[i]);
- key[i] = 0;
+ key = strdup(key_);
+ for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
+ *tl = tolower(*tl);
if (use_key_regexp) {
key_regexp = (regex_t*)malloc(sizeof(regex_t));
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 7090ea9..f341206 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -23,6 +23,7 @@ git-repo-config Core.Movie BadPhysics
cat > expect << EOF
[core]
penguin = little blue
+[Core]
Movie = BadPhysics
EOF
@@ -33,6 +34,7 @@ git-repo-config Cores.WhatEver Second
cat > expect << EOF
[core]
penguin = little blue
+[Core]
Movie = BadPhysics
[Cores]
WhatEver = Second
@@ -45,10 +47,12 @@ git-repo-config CORE.UPPERCASE true
cat > expect << EOF
[core]
penguin = little blue
+[Core]
Movie = BadPhysics
- UPPERCASE = true
[Cores]
WhatEver = Second
+[CORE]
+ UPPERCASE = true
EOF
test_expect_success 'similar section' 'cmp .git/config expect'
@@ -62,11 +66,13 @@ test_expect_success 'replace with non-ma
cat > expect << EOF
[core]
penguin = very blue
- Movie = BadPhysics
- UPPERCASE = true
penguin = kingpin
+[Core]
+ Movie = BadPhysics
[Cores]
WhatEver = Second
+[CORE]
+ UPPERCASE = true
EOF
test_expect_success 'non-match result' 'cmp .git/config expect'
@@ -130,7 +136,7 @@ EOF
test_expect_success 'really mean test' 'cmp .git/config expect'
-git-repo-config nextsection.nonewline wow
+git-repo-config nextSection.nonewline wow
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -160,7 +166,7 @@ EOF
test_expect_success 'unset' 'cmp .git/config expect'
-git-repo-config nextsection.NoNewLine "wow2 for me" "for me$"
+git-repo-config nextSection.NoNewLine "wow2 for me" "for me$"
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -176,18 +182,18 @@ EOF
test_expect_success 'multivar' 'cmp .git/config expect'
test_expect_success 'non-match' \
- 'git-repo-config --get nextsection.nonewline !for'
+ 'git-repo-config --get nextSection.nonewline !for'
test_expect_success 'non-match value' \
- 'test wow = $(git-repo-config --get nextsection.nonewline !for)'
+ 'test wow = $(git-repo-config --get nextSection.nonewline !for)'
test_expect_failure 'ambiguous get' \
- 'git-repo-config --get nextsection.nonewline'
+ 'git-repo-config --get nextSection.nonewline'
test_expect_success 'get multivar' \
- 'git-repo-config --get-all nextsection.nonewline'
+ 'git-repo-config --get-all nextSection.nonewline'
-git-repo-config nextsection.nonewline "wow3" "wow$"
+git-repo-config nextSection.nonewline "wow3" "wow$"
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -202,15 +208,15 @@ EOF
test_expect_success 'multivar replace' 'cmp .git/config expect'
-test_expect_failure 'ambiguous value' 'git-repo-config nextsection.nonewline'
+test_expect_failure 'ambiguous value' 'git-repo-config nextSection.nonewline'
test_expect_failure 'ambiguous unset' \
- 'git-repo-config --unset nextsection.nonewline'
+ 'git-repo-config --unset nextSection.nonewline'
test_expect_failure 'invalid unset' \
- 'git-repo-config --unset somesection.nonewline'
+ 'git-repo-config --unset someSection.nonewline'
-git-repo-config --unset nextsection.nonewline "wow3$"
+git-repo-config --unset nextSection.nonewline "wow3$"
cat > expect << EOF
[beta] ; silly comment # another comment
@@ -249,7 +255,7 @@ test_expect_success 'hierarchical sectio
cat > expect << EOF
beta.noindent=sillyValue
-nextsection.nonewline=wow2 for me
+nextSection.nonewline=wow2 for me
123456.a123=987
1.2.3.alpha=beta
EOF
@@ -259,7 +265,7 @@ test_expect_success 'working --list' \
cat > expect << EOF
beta.noindent sillyValue
-nextsection.nonewline wow2 for me
+nextSection.nonewline wow2 for me
EOF
test_expect_success '--get-regexp' \
^ permalink raw reply related
* [PATCH 0/2] Support for "set -e" in cogito script, and assertion in rewritehist
From: Yann Dirson @ 2006-05-09 22:27 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
The first patch is an updated version of the one posted on Apr 17th, which makes
cg-Xlib compatible with "set -e" for safer scripts, and converts cg-admin-rewritehist
to "set -e" mode. The second ones makes the latter robust on one backquoted command,
for which "set -e" is not sufficient. Let the devil bring shell programming into hell,
asap.
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
http://ydirson.free.fr/ | Check <http://www.debian.org/>
^ permalink raw reply
* [PATCH 2/2] Catch history inconsistency in cg-admin-rewritehist.
From: Yann Dirson @ 2006-05-09 22:32 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20060509222738.20814.57282.stgit@gandelf.nowhere.earth>
This assertion is triggered by a bug in "cg-object-id -p", which
ignores grafts, when we attempt to rewrite a grafted commit whose
original parent does not get rewritten.
Dying here prevents to leave the user with a corrupted rewriten history.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
cg-admin-rewritehist | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/cg-admin-rewritehist b/cg-admin-rewritehist
index 958a8ab..8dd33f2 100755
--- a/cg-admin-rewritehist
+++ b/cg-admin-rewritehist
@@ -218,9 +218,13 @@ while read commit; do
parentstr=
for parent in $(cg-object-id -p $commit); do
- for reparent in $(cat ../map/$parent); do
- parentstr="$parentstr -p $reparent"
- done
+ if [ -r "../map/$parent" ]; then
+ for reparent in $(cat "../map/$parent"); do
+ parentstr="$parentstr -p $reparent"
+ done
+ else
+ die "assertion failed: parent $parent for commit $commit not found in rewritten ones"
+ fi
done
if [ "$filter_parent" ]; then
parentstr="$(echo "$parentstr" | eval "$filter_parent")"
^ permalink raw reply related
* [PATCH 1/2] Suppress use of unsafe idiomatic use of && in cg-Xlib
From: Yann Dirson @ 2006-05-09 22:32 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20060509222738.20814.57282.stgit@gandelf.nowhere.earth>
This is a necessary step to make any cogito script "set -e"-safe.
Also fixes similar issues in cg-admin-rewritehist and turn "set -e" on
there.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
cg-Xlib | 82 ++++++++++++++++++++++++++------------------------
cg-admin-rewritehist | 8 +++--
2 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/cg-Xlib b/cg-Xlib
index 31f93d2..ff2b895 100755
--- a/cg-Xlib
+++ b/cg-Xlib
@@ -26,7 +26,7 @@ warn()
fi
echo "Warning: $@" >&2
- [ "$beep" ] && echo -ne "\a" >&2
+ [ -z "$beep" ] || echo -ne "\a" >&2
}
die()
@@ -177,7 +177,7 @@ showdate()
{
local secs=$1 tzhours=${2:0:3} tzmins=${2:0:1}${2:3} format="$3"
# bash doesn't like leading zeros
- [ "${tzhours:1:1}" = 0 ] && tzhours=${2:0:1}${2:2:1}
+ [ "${tzhours:1:1}" != 0 ] || tzhours=${2:0:1}${2:2:1}
secs=$(($secs + $tzhours * 3600 + $tzmins * 60))
[ "$format" ] || format="+%a, %d %b %Y %H:%M:%S $2"
if [ "$has_gnudate" ]; then
@@ -221,7 +221,7 @@ colorify_diffcolors="$colorify_diffcolor
colorify_setup()
{
local C="$1"
- [ -n "$CG_COLORS" ] && C="$C:$CG_COLORS"
+ [ -z "$CG_COLORS" ] || C="$C:$CG_COLORS"
C=${C//=/=\'$'\e'[}
C=col${C//:/m\'; col}m\'
@@ -261,7 +261,7 @@ column_width()
done | sort -nr | head -n 1 |
(
read maxlen;
- [ ${maxlen:-0} -gt $maxwidth ] && maxlen=$maxwidth;
+ [ ${maxlen:-0} -le $maxwidth ] || maxlen=$maxwidth;
echo ${maxlen:-0}
)
}
@@ -283,7 +283,7 @@ columns_print()
else
fmt="$fmt%-${width}s"
fi
- [ "$tab" ] && fmt="$fmt\t";
+ [ -z "$tab" ] || fmt="$fmt\t";
done
printf "$fmt\n" "${cols[@]}"
}
@@ -321,7 +321,7 @@ pick_id()
'
LANG=C LC_ALL=C sed -ne "$pick_id_script"
# Ensure non-empty id name.
- echo "[ -z \"\$GIT_${uid}_NAME\" ] && export GIT_${uid}_NAME=\"\${GIT_${uid}_EMAIL%%@*}\""
+ echo "[ -n \"\$GIT_${uid}_NAME\" ] || export GIT_${uid}_NAME=\"\${GIT_${uid}_EMAIL%%@*}\""
}
pick_author()
@@ -355,16 +355,16 @@ while IFS= read -r inp; do
done
path[${#path[@]}]="$inp"
for (( i=0; $i < ${#path[@]}; i++ )); do
- [ "${path[$i]}" = "." ] && continue
+ [ "${path[$i]}" != "." ] || continue
if [ "${path[$i]}" = ".." ]; then
- [ "${#path2[@]}" -gt 0 ] && unset path2[$((${#path2[@]} - 1))]
+ [ "${#path2[@]}" -le 0 ] || unset path2[$((${#path2[@]} - 1))]
continue
fi
path2[${#path2[@]}]="${path[$i]}"
done
for (( i=0; $i < ${#path2[@]}; i++ )); do
echo -n "${path2[$i]}"
- [ $i -lt $((${#path2[@]} - 1)) ] && echo -n /
+ [ $i -ge $((${#path2[@]} - 1)) ] || echo -n /
done
echo
done
@@ -400,7 +400,7 @@ # only the dirname/ will be
# EXTRAEXCLUDE: extra exclude pattern
list_untracked_files()
{
- [ "$_git_no_wc" ] && die "INTERNAL ERROR: list_untracked_files() outside a working copy"
+ [ -z "$_git_no_wc" ] || die "INTERNAL ERROR: list_untracked_files() outside a working copy"
excludeflag="$1"; shift
squashflag="$1"; shift
EXCLUDE=()
@@ -443,7 +443,7 @@ list_untracked_files()
fi
fi
local listdirs=
- [ "$squashflag" = "squashdirs" ] && listdirs=--directory
+ [ "$squashflag" != "squashdirs" ] || listdirs=--directory
git-ls-files -z --others $listdirs "${EXCLUDE[@]}"
}
@@ -512,8 +512,8 @@ editor()
actionkey="$1"; shift
${EDITOR:-vi} "$LOGMSG2"
- [ "$force" ] && return 0
- [ "$LOGMSG2" -nt "$LOGMSG" ] && return 0
+ [ -z "$force" ] || return 0
+ [ "$LOGMSG" -nt "$LOGMSG2" ] || return 0
echo "Log message unchanged or not specified" >&2
while true; do
@@ -538,7 +538,7 @@ # the working copy (if ROLLBACK_BOOL) an
# Returns false in case of local modifications (but only if ROLLBACK_ROLL).
tree_timewarp()
{
- [ "$_git_no_wc" ] && die "INTERNAL ERROR: tree_timewarp() outside a working copy"
+ [ -z "$_git_no_wc" ] || die "INTERNAL ERROR: tree_timewarp() outside a working copy"
local no_head_update=
if [ "$1" = "--no-head-update" ]; then
no_head_update=1
@@ -550,14 +550,14 @@ tree_timewarp()
local branch="$1"; shift
local localmods=0
- [ -s "$_git/merging" ] && die "merge in progress - cancel it by cg-reset first"
+ [ ! -s "$_git/merging" ] || die "merge in progress - cancel it by cg-reset first"
if [ -n "$rollback" ]; then
local patchfile="$(mktemp -t gituncommit.XXXXXX)"
cg-diff -r "$base" >"$patchfile"
- [ -s "$patchfile" ] &&
+ [ ! -s "$patchfile" ] ||
warn "uncommitted local changes, trying to bring them $dirstr"
- [ -s "$patchfile" ] && localmods=1
+ [ ! -s "$patchfile" ] || localmods=1
git-read-tree -m "$branch" || die "$branch: bad commit"
[ "$no_head_update" ] || git-update-ref HEAD "$branch" || :
@@ -589,9 +589,9 @@ conservative_merge_base()
_cg_base_conservative=
for (( safecounter=0; $safecounter < 1000; safecounter++ )) ; do
baselist=($(git-merge-base --all "${baselist[@]}")) || return 1
- [ "${#baselist[@]}" -le "1" ] && break
+ [ "${#baselist[@]}" -gt "1" ] || break
done
- [ $safecounter -gt 0 ] && _cg_base_conservative=$safecounter
+ [ $safecounter -le 0 ] || _cg_base_conservative=$safecounter
_cg_baselist=("${baselist[@]}")
}
@@ -603,7 +603,7 @@ # Never use it. If you do, accompany it
# it safe to use it.
update_index()
{
- [ "$_git_no_wc" ] && die "INTERNAL ERROR: update_index() outside a working copy"
+ [ -z "$_git_no_wc" ] || die "INTERNAL ERROR: update_index() outside a working copy"
git-update-index --refresh | sed 's/needs update$/locally modified/'
}
@@ -620,14 +620,14 @@ is_same_repo()
# second side...
if [ ! -w "$dir1" -o ! -w "$dir2" ]; then
# ...except in readonly setups.
- [ "$(readlink -f "$dir1")" = "$(readlink -f "$dir2")" ] && diff=0
+ [ "$(readlink -f "$dir1")" != "$(readlink -f "$dir2")" ] || diff=0
else
n=$$
while [ -e "$dir1/.,,lnstest-$n" -o -e "$dir2/.,,lnstest-$n" ]; do
n=$((n+1))
done
touch "$dir1/.,,lnstest-$n"
- [ -e "$dir2/.,,lnstest-$n" ] && diff=0
+ [ ! -e "$dir2/.,,lnstest-$n" ] || diff=0
rm "$dir1/.,,lnstest-$n"
fi
return $diff
@@ -679,7 +679,7 @@ deprecated_alias()
cmd="${0##*/}"
propername="$1"; shift
for a in "$@"; do
- [ "$cmd" = "$a" ] && \
+ [ "$cmd" != "$a" ] || \
warn "'$a' is a deprecated alias, please use '$propername' instead"
done
}
@@ -709,7 +709,7 @@ print_help()
echo
echo "Options:"
maxlen="$(sed -n 's/^# \(-.*\)::[^A-Za-z0-9].*/\1/p' < "$_cg_cmd" | column_width)"
- [ $maxlen -lt 11 ] && maxlen=11 # --long-help
+ [ $maxlen -ge 11 ] || maxlen=11 # --long-help
_cg_fmt=" %-20s %s\n"
sed -n 's/# \(-.*\)::[^A-Za-z0-9]\(.*\)/\1\n\2/p' < "$_cg_cmd" | while read line; do
case "$line" in
@@ -727,7 +727,7 @@ print_help()
}
for option in "$@"; do
- [ x"$option" = x-- ] && break
+ [ x"$option" != x-- ] || break
if [ x"$option" = x"-h" ] || [ x"$option" = x"--help" ]; then
print_help short "${_cg_cmd##cg-}"
elif [ x"$option" = x"--long-help" ]; then
@@ -777,8 +777,8 @@ optparse()
--) optshift; return 1 ;;
-*) return 0 ;;
*) while (( ++ARGPOS < ${#ARGS[@]} )); do
- [[ "${ARGS[$ARGPOS]}" == -- ]] && return 1
- [[ "${ARGS[$ARGPOS]}" == -* ]] && return 0
+ [[ "${ARGS[$ARGPOS]}" != -- ]] || return 1
+ [[ "${ARGS[$ARGPOS]}" != -* ]] || return 0
done;
return 1 ;;
esac
@@ -786,22 +786,26 @@ optparse()
CUROPT="${ARGS[$ARGPOS]}"
local match="${1%=}" minmatch="${2:-1}" opt="$CUROPT" o="$CUROPT" val
- [[ "$1" == *= ]] && val="$match"
+ [[ "$1" != *= ]] || val="$match"
case "$match" in
--*)
- [ "$val" ] && o="${o%%=*}"
+ [ -z "$val" ] || o="${o%%=*}"
[ ${#o} -ge $((2 + $minmatch)) -a \
"${match:0:${#o}}" = "$o" ] || return 1
- [[ -n "$val" && "$opt" == *=?* ]] \
- && ARGS[$ARGPOS]="${opt#*=}" \
- || optshift "$val" ;;
+ if [[ -n "$val" && "$opt" == *=?* ]]; then
+ ARGS[$ARGPOS]="${opt#*=}"
+ else
+ optshift "$val"
+ fi ;;
-?)
[[ "$o" == $match* ]] || return 1
[[ "$o" != -?-* || -n "$val" ]] || optfail
ARGS[$ARGPOS]=${o#$match}
- [ -n "${ARGS[$ARGPOS]}" ] \
- && { [ -n "$val" ] || ARGS[$ARGPOS]=-"${ARGS[$ARGPOS]}"; } \
- || optshift "$val" ;;
+ if [ -n "${ARGS[$ARGPOS]}" ]; then
+ [ -n "$val" ] || ARGS[$ARGPOS]=-"${ARGS[$ARGPOS]}";
+ else
+ optshift "$val"
+ fi ;;
*)
die "optparse cannot handle $1" ;;
esac
@@ -846,7 +850,7 @@ check_tool()
fi
done
IFS="$save_IFS"
- [ "$hasname" ] && break
+ [ -z "$hasname" ] || break
done 2>/dev/null
}
@@ -882,7 +886,7 @@ if [ ! "$_git_repo_unneeded" ]; then
_git=.
export GIT_DIR=.
fi
- [ "$GIT_DIR" = . ] && _git_no_wc=1
+ [ "$GIT_DIR" != . ] || _git_no_wc=1
if [ ! -d "$_git" ]; then
echo "There is no GIT repository here ($_git not found)" >&2
exit 1
@@ -894,8 +898,8 @@ if [ ! "$_git_repo_unneeded" ]; then
exit 1
fi
_git_head=master
- [ -s "$_git/HEAD" ] && { _git_head="$(git-symbolic-ref HEAD)"; _git_head="${_git_head#refs/heads/}"; }
- [ -s "$_git/head-name" ] && _git_head="$(cat "$_git/head-name")"
+ [ ! -s "$_git/HEAD" ] || { _git_head="$(git-symbolic-ref HEAD)"; _git_head="${_git_head#refs/heads/}"; }
+ [ ! -s "$_git/head-name" ] || _git_head="$(cat "$_git/head-name")"
fi
# Check if the script requires to be called from the workdir root.
diff --git a/cg-admin-rewritehist b/cg-admin-rewritehist
index 9fa4c2a..958a8ab 100755
--- a/cg-admin-rewritehist
+++ b/cg-admin-rewritehist
@@ -133,6 +133,8 @@ # as their parents instead of the merge
# Testsuite: TODO
+set -e
+
USAGE="cg-admin-rewritehist [-d TEMPDIR] [-r STARTREV]... [FILTERS] DESTBRANCH"
_git_wc_unneeded=1
_git_requires_root=1
@@ -169,10 +171,10 @@ done
dstbranch="${ARGS[0]}"
[ -n "$dstbranch" ] || die "missing branch name"
-[ -s "$_git/refs/heads/$dstbranch" ] && die "branch $dstbranch already exists"
-[ -s "$_git/branches/$dstbranch" ] && die "branch $dstbranch is already a remote branch"
+[ ! -s "$_git/refs/heads/$dstbranch" ] || die "branch $dstbranch already exists"
+[ ! -s "$_git/branches/$dstbranch" ] || die "branch $dstbranch is already a remote branch"
-[ -e "$tempdir" ] && die "$tempdir already exists, please remove it"
+[ ! -e "$tempdir" ] || die "$tempdir already exists, please remove it"
mkdir -p "$tempdir/t"
cd "$tempdir/t"
^ permalink raw reply related
* Re: Implementing branch attributes in git config
From: Junio C Hamano @ 2006-05-09 22:42 UTC (permalink / raw)
To: sean; +Cc: torvalds, junkio, Johannes.Schindelin, git
In-Reply-To: <BAYC1-PASMTP04D623089E043F1C792A37AEA90@CEZ.ICE>
sean <seanlkml@sympatico.ca> writes:
> The syntax is:
>
> [<random string>]
Does this mean you can have anything other than LF and ']'?
> Here's how your example would look in this style:
>
> [email.torvalds@osdl.org]
> name = Linus Torvalds
How does a program (not a script, but git_config() users) get
that value and parse it?
^ permalink raw reply
* [PATCH] Fix cg-object-id to lookup parents in the Right Way
From: Yann Dirson @ 2006-05-09 22:49 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
This causes grafts to not be ignored any more, allowing
cg-admin-rewritehist to work as expected on grafted commits.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
cg-object-id | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/cg-object-id b/cg-object-id
index ee23082..71c72e5 100755
--- a/cg-object-id
+++ b/cg-object-id
@@ -147,7 +147,7 @@ fi
if [ "$show_parent" ]; then
- git-cat-file commit "$normid" | sed -ne 's/^parent //p;/^$/q'
+ git-rev-list --parents -n 1 "$normid"| tr ' ' '\n'|tail +2
exit 0
fi
^ permalink raw reply related
* Re: Implementing branch attributes in git config
From: sean @ 2006-05-09 22:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: torvalds, junkio, Johannes.Schindelin, git
In-Reply-To: <7vu07y252m.fsf@assigned-by-dhcp.cox.net>
On Tue, 09 May 2006 15:42:25 -0700
Junio C Hamano <junkio@cox.net> wrote:
> sean <seanlkml@sympatico.ca> writes:
>
> > The syntax is:
> >
> > [<random string>]
>
> Does this mean you can have anything other than LF and ']'?
Anything but LF; how's this for ugly:
["hello Worl\]d \\backslash]
> > Here's how your example would look in this style:
> >
> > [email.torvalds@osdl.org]
> > name = Linus Torvalds
>
> How does a program (not a script, but git_config() users) get
> that value and parse it?
The same way they do now. For instance git-repo-config processes
the config file using the same get_config() + callback as usual. The
only issue is that they should no longer cast everything to lower first.
Sean
^ permalink raw reply
* Re: Implementing branch attributes in git config
From: sean @ 2006-05-09 23:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: torvalds, Johannes.Schindelin, git
In-Reply-To: <20060509184519.5a707231.seanlkml@sympatico.ca>
On Tue, 9 May 2006 18:45:19 -0400
sean <seanlkml@sympatico.ca> wrote:
> > How does a program (not a script, but git_config() users) get
> > that value and parse it?
>
> The same way they do now. For instance git-repo-config processes
> the config file using the same get_config() + callback as usual. The
> only issue is that they should no longer cast everything to lower first.
Junio,
Sorry I see what you're driving at; how does a program break the section
name into it's constituent pieces. I glossed over this issue because
it's exactly the same between Linus' proposal and mine. The answer is,
they really can't, with either proposal.
All you can count on (by convention) is that there is an initial segment
that is terminated by a period; and a final segment that starts with a
period, and everything in between is an opaque unit.
section.<random string>.keyname value
Although the initial "section." isn't currently enforced (but easily
could be). Actually i didn't yank out the bit from config.c that
validates the keyname, so without an additional patch the only way
to enter the extended names is by manual editing of the .git/config.
Sean
^ permalink raw reply
* Re: Implementing branch attributes in git config
From: Pavel Roskin @ 2006-05-09 23:23 UTC (permalink / raw)
To: sean; +Cc: Junio C Hamano, torvalds, Johannes.Schindelin, git
In-Reply-To: <20060509184519.5a707231.seanlkml@sympatico.ca>
On Tue, 2006-05-09 at 18:45 -0400, sean wrote:
> On Tue, 09 May 2006 15:42:25 -0700
> Junio C Hamano <junkio@cox.net> wrote:
> > Does this mean you can have anything other than LF and ']'?
>
> Anything but LF; how's this for ugly:
>
> ["hello Worl\]d \\backslash]
Actually, LF is already handled just fine in the value part:
[proski@dv .git]$ git-repo-config s1.k1 $'v1\nv2'
[proski@dv .git]$ grep [sk]1 config
[s1]
k1 = v1\nv2
Note that quoting doesn't solve this problem (unless multi-line section
headers are allowed), but backslash escaping does.
But I guess everybody prefers quotes for their friendly look.
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: Implementing branch attributes in git config
From: Linus Torvalds @ 2006-05-10 0:17 UTC (permalink / raw)
To: sean; +Cc: junkio, Johannes.Schindelin, git
In-Reply-To: <BAYC1-PASMTP04D623089E043F1C792A37AEA90@CEZ.ICE>
On Tue, 9 May 2006, sean wrote:
>
> I really tried to like your patch ;o) But it breaks the repo-config command
> line and causes mixing of some branches using old style [branch.xyz] and new
> style [branch "XYZ"] which just doesn't seem to be the right thing to do.
Well, obviously it's _simpler_ to have the machine-readable format as-is,
and say "don't try to make it human-readable". But the repo-config patch
to make it human-readable isn't that hard.
And it's _not_ that hard to make repo-config do the right thing.
Here's a pretty lightly tested patch (on top of my previous one) that does
exactly that.
(The first part just fixes an indentation bug)
Linus
---
diff --git a/config.c b/config.c
index f3b74e0..12c51b1 100644
--- a/config.c
+++ b/config.c
@@ -372,10 +372,12 @@ static int store_aux(const char* key, co
store.offset[store.seen] = ftell(config_file);
store.state = KEY_SEEN;
store.seen++;
- } else if (strrchr(key, '.') - key == store.baselen &&
+ } else {
+ if (strrchr(key, '.') - key == store.baselen &&
!strncmp(key, store.key, store.baselen)) {
store.state = SECTION_SEEN;
store.offset[store.seen] = ftell(config_file);
+ }
}
}
return 0;
@@ -383,8 +385,30 @@ static int store_aux(const char* key, co
static void store_write_section(int fd, const char* key)
{
+ const char *dot = strchr(key, '.');
+ int len1 = store.baselen, len2 = -1;
+
+ dot = strchr(key, '.');
+ if (dot) {
+ int dotlen = dot - key;
+ if (dotlen < len1) {
+ len2 = len1 - dotlen - 1;
+ len1 = dotlen;
+ }
+ }
+
write(fd, "[", 1);
- write(fd, key, store.baselen);
+ write(fd, key, len1);
+ if (len2 >= 0) {
+ write(fd, " \"", 2);
+ while (--len2 >= 0) {
+ unsigned char c = *++dot;
+ if (c == '"')
+ write(fd, "\\", 1);
+ write(fd, &c, 1);
+ }
+ write(fd, "\"", 1);
+ }
write(fd, "]\n", 2);
}
@@ -458,7 +482,7 @@ int git_config_set(const char* key, cons
int git_config_set_multivar(const char* key, const char* value,
const char* value_regex, int multi_replace)
{
- int i;
+ int i, dot;
int fd = -1, in_fd;
int ret;
char* config_filename = strdup(git_path("config"));
@@ -483,16 +507,23 @@ int git_config_set_multivar(const char*
* Validate the key and while at it, lower case it for matching.
*/
store.key = (char*)malloc(strlen(key)+1);
- for (i = 0; key[i]; i++)
- if (i != store.baselen &&
- ((!isalnum(key[i]) && key[i] != '.') ||
- (i == store.baselen+1 && !isalpha(key[i])))) {
- fprintf(stderr, "invalid key: %s\n", key);
- free(store.key);
- ret = 1;
- goto out_free;
- } else
- store.key[i] = tolower(key[i]);
+ dot = 0;
+ for (i = 0; key[i]; i++) {
+ unsigned char c = key[i];
+ if (c == '.')
+ dot = 1;
+ /* Leave the extended basename untouched.. */
+ if (!dot || i > store.baselen) {
+ if (!isalnum(c) || (i == store.baselen+1 && !isalpha(c))) {
+ fprintf(stderr, "invalid key: %s\n", key);
+ free(store.key);
+ ret = 1;
+ goto out_free;
+ }
+ c = tolower(c);
+ }
+ store.key[i] = c;
+ }
store.key[i] = 0;
/*
^ permalink raw reply related
* Re: Implementing branch attributes in git config
From: sean @ 2006-05-10 1:38 UTC (permalink / raw)
To: Linus Torvalds; +Cc: junkio, Johannes.Schindelin, git
In-Reply-To: <Pine.LNX.4.64.0605091543100.3718@g5.osdl.org>
On Tue, 9 May 2006 17:17:58 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:
> Here's a pretty lightly tested patch (on top of my previous one) that does
> exactly that.
This patch or something like it is needed for repo-config in order to query
the new case sensitive portion of section names. This is on top of your two
patches:
diff --git a/repo-config.c b/repo-config.c
index 63eda1b..9a9194f 100644
--- a/repo-config.c
+++ b/repo-config.c
@@ -65,11 +65,14 @@ static int show_config(const char* key_,
static int get_value(const char* key_, const char* regex_)
{
int i;
+ char *tl;
+
+ key = strdup(key_);
+ for (tl=key+strlen(key)-1; tl >= key && *tl != '.'; --tl)
+ *tl = tolower(*tl);
+ for (tl=key; *tl && *tl != '.'; ++tl)
+ *tl = tolower(*tl);
- key = malloc(strlen(key_)+1);
- for (i = 0; key_[i]; i++)
- key[i] = tolower(key_[i]);
- key[i] = 0;
if (use_key_regexp) {
key_regexp = (regex_t*)malloc(sizeof(regex_t));
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox