[PATCH 1/2] Allow '-' in config variable names

[PATCH 1/2] Allow '-' in config variable names

[Linus Torvalds]

I need this in order to allow aliases of the same form as "ls-tree", 
"rev-parse" etc, so that I can use

	[alias]
		my-cat=--paginate cat-file -p

to add a "git my-cat" command.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

This will be followed by another patch to "builtin-push.c" to allow me to 
make a useful (to me) "push-all" alias.

diff --git a/config.c b/config.c
index e8f0caf..3cae390 100644
--- a/config.c
+++ b/config.c
@@ -103,6 +103,11 @@ static char *parse_value(void)
 	}
 }
 
+static inline int iskeychar(int c)
+{
+	return isalnum(c) || c == '-';
+}
+
 static int get_value(config_fn_t fn, char *name, unsigned int len)
 {
 	int c;
@@ -113,7 +118,7 @@ static int get_value(config_fn_t fn, cha
 		c = get_next_char();
 		if (c == EOF)
 			break;
-		if (!isalnum(c))
+		if (!iskeychar(c))
 			break;
 		name[len++] = tolower(c);
 		if (len >= MAXNAME)
@@ -181,7 +186,7 @@ static int get_base_var(char *name)
 			return baselen;
 		if (isspace(c))
 			return get_extended_base_var(name, baselen, c);
-		if (!isalnum(c) && c != '.')
+		if (!iskeychar(c) && c != '.')
 			return -1;
 		if (baselen > MAXNAME / 2)
 			return -1;
@@ -573,7 +578,7 @@ int git_config_set_multivar(const char*
 			dot = 1;
 		/* Leave the extended basename untouched.. */
 		if (!dot || i > store.baselen) {
-			if (!isalnum(c) || (i == store.baselen+1 && !isalpha(c))) {
+			if (!iskeychar(c) || (i == store.baselen+1 && !isalpha(c))) {
 				fprintf(stderr, "invalid key: %s\n", key);
 				free(store.key);

[PATCH 2/2] git push: add verbose flag and allow overriding of default target repository

[Linus Torvalds]

This adds a command line flag "-v" to enable a more verbose mode, and 
"--repo=" to override the default target repository for "git push" (which 
otherwise always defaults to "origin").

This, together with the patch to allow dashes in config variable names, 
allows me to do

	[alias]
		push-all = push -v --repo=all

in my user-global config file, and then I can (for any project I maintain) 
add to the project-local config file

	[remote "all"]
		url=one.target.repo:/directory
		url=another.target:/pub/somewhere/else

and now "git push-all" just updates all the target repositories, and shows 
me what it does - regardless of which repo I am in.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

Maybe this is just useful to me? I dunno. I have long had a per-repository 
"push-all" script that I have in the root directory of the repos I 
maintain, but this is much more practical. With this, I can do things like

	git push-all --tags

and it will push all the new tags to all the public repositories I 
maintain for that particular repository.

Special Linus-only behaviour? Maybe. On the other hand, I think this is a 
pretty clean patch regardless.

diff --git a/builtin-push.c b/builtin-push.c
index f5150ed..3151fb7 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,7 +10,7 @@
 
 static const char push_usage[] = "git-push [--all] [--tags] [-f | --force] <repository> [<refspec>...]";
 
-static int all, tags, force, thin = 1;
+static int all, tags, force, thin = 1, verbose;
 static const char *execute;
 
 #define BUF_SIZE (2084)
@@ -248,6 +248,8 @@ static int do_push(const char *repo)
 		while (dest_refspec_nr--)
 			argv[dest_argc++] = *dest_refspec++;
 		argv[dest_argc] = NULL;
+		if (verbose)
+			fprintf(stderr, "Pushing to %s\n", dest);
 		err = run_command_v(argc, argv);
 		if (!err)
 			continue;
@@ -281,6 +283,14 @@ int cmd_push(int argc, const char **argv
 			i++;
 			break;
 		}
+		if (!strcmp(arg, "-v")) {
+			verbose=1;
+			continue;
+		}
+		if (!strncmp(arg, "--repo=", 7)) {
+			repo = arg+7;
+			continue;
+		}
 		if (!strcmp(arg, "--all")) {
 			all = 1;

Re: [PATCH 1/2] Allow '-' in config variable names

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> I need this in order to allow aliases of the same form as "ls-tree", 
> "rev-parse" etc, so that I can use
>
> 	[alias]
> 		my-cat=--paginate cat-file -p
>
> to add a "git my-cat" command.

I do not have problem with this (and would perhaps also want to
add '_' to keychar set), but people who envisioned parsing
config from scripts (i.e. Perly git) might prefer if we stayed
within alnum, since I'd suspect then they may be able to reuse
existing .ini parsers.  I do not much care about that myself,
but I am bringing it up just in case other people might.

Other than that, this sounds nice.

By the way, everybody seems to do "alias.xxx = -p cat-file -p"
(I have it as "git less").  Maybe we would want to make a
built-in alias for that?

Re: [PATCH 2/2] git push: add verbose flag and allow overriding of default target repository

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> This adds a command line flag "-v" to enable a more verbose mode, and 
> "--repo=" to override the default target repository for "git push" (which 
> otherwise always defaults to "origin").
>
> This, together with the patch to allow dashes in config variable names, 
> allows me to do
>
> 	[alias]
> 		push-all = push -v --repo=all
>
> in my user-global config file, and then I can (for any project I maintain) 
> add to the project-local config file
>
> 	[remote "all"]
> 		url=one.target.repo:/directory
> 		url=another.target:/pub/somewhere/else
>
> and now "git push-all" just updates all the target repositories, and shows 
> me what it does - regardless of which repo I am in.
>
> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> ---
>
> Maybe this is just useful to me?  I dunno.

I would say if something is useful to you it is useful to your
subsystem people and anybody who has a public tree and more than
one machines to verify the tips of his branches on, including
me.

Re: [PATCH 1/2] Allow '-' in config variable names

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Linus Torvalds <torvalds@osdl.org> writes:
>
>> I need this in order to allow aliases of the same form as "ls-tree", 
>> "rev-parse" etc, so that I can use
>>
>> 	[alias]
>> 		my-cat=--paginate cat-file -p
>>
>> to add a "git my-cat" command.
>
> I do not have problem with this (and would perhaps also want to
> add '_' to keychar set), but people who envisioned parsing
> config from scripts (i.e. Perly git) might prefer if we stayed
> within alnum, since I'd suspect then they may be able to reuse
> existing .ini parsers.  I do not much care about that myself,
> but I am bringing it up just in case other people might.
>
> Other than that, this sounds nice.

One thing I forgot to add.  Just like we downcase what user has
written in config file, it might make sense to also remove '-'
(and if we add '_' to keychar set, that one as well) to when
canonicalizing the key value.  That way, somewhat awkward long
configuration variables we currently have can be written more
readably, e.g. repack.use-delta-base-offset

Likes, dislikes?  It is not strictly needed, since we can do
CamelCase as well in the configuration file.

> By the way, everybody seems to do "alias.xxx = -p cat-file -p"
> (I have it as "git less").  Maybe we would want to make a
> built-in alias for that?

Seconds?

Re: [PATCH 1/2] Allow '-' in config variable names

[J. Bruce Fields]

On Mon, Oct 30, 2006 at 07:02:27PM -0800, Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> > By the way, everybody seems to do "alias.xxx = -p cat-file -p"
> > (I have it as "git less").  Maybe we would want to make a
> > built-in alias for that?
> 
> Seconds?

Yes!

I'd vote for calling it just "git cat".

Re: [PATCH 2/2] git push: add verbose flag and allow overriding of default target repository

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> diff --git a/builtin-push.c b/builtin-push.c
> index f5150ed..3151fb7 100644
> --- a/builtin-push.c
> +++ b/builtin-push.c
> @@ -10,7 +10,7 @@
>  
>  static const char push_usage[] = "git-push [--all] [--tags] [-f | --force] <repository> [<refspec>...]";
>  
> -static int all, tags, force, thin = 1;
> +static int all, tags, force, thin = 1, verbose;
>  static const char *execute;
>  
>  #define BUF_SIZE (2084)

By the way, has anybody noticed we use a funny value for the
buffer size?

Re: [PATCH 1/2] Allow '-' in config variable names

[Linus Torvalds]

On Mon, 30 Oct 2006, Junio C Hamano wrote:
> 
> One thing I forgot to add.  Just like we downcase what user has
> written in config file, it might make sense to also remove '-'
> (and if we add '_' to keychar set, that one as well) to when
> canonicalizing the key value.  That way, somewhat awkward long
> configuration variables we currently have can be written more
> readably, e.g. repack.use-delta-base-offset

Well, then we need to teach "git <alias>" that "git <al-i-as>" is the same 
thing as "alias".

> > By the way, everybody seems to do "alias.xxx = -p cat-file -p"
> > (I have it as "git less").  Maybe we would want to make a
> > built-in alias for that?
> 
> Seconds?

I like it mainly as a great example of how to use aliases, not because I 
actually much -use- the thing, so I don't personally care.

Re: [PATCH 1/2] Allow '-' in config variable names

[Johannes Schindelin]

Hi,

On Mon, 30 Oct 2006, Junio C Hamano wrote:

> Junio C Hamano <junkio@cox.net> writes:
> 
> > Linus Torvalds <torvalds@osdl.org> writes:
> >
> >> I need this in order to allow aliases of the same form as "ls-tree", 
> >> "rev-parse" etc, so that I can use
> >>
> >> 	[alias]
> >> 		my-cat=--paginate cat-file -p
> >>
> >> to add a "git my-cat" command.
> >
> Seconds?

I personally dislike any name with "_" or "-" in it, since I am stuck with 
different keyboard layouts and keep mistyping them. I even often find 
myself hitting <TAB> just to complete a _single_ "-", since the position 
of the <TAB> key is not wildly jumping around between different keyboard 
layouts.

So I can live without it.

Ciao,
Dscho