[PATCH] allow user aliases for the --author parameter

This allows the use of author abbreviations when specifying commit
authors via the --author option to git commit. "--author=$key" is
resolved by looking up "user.$key.name" and "user.$key.email" in the
config.

Signed-off-by: Michael J Gruber <redacted>
---
In an ideal word, all my collaborators would exchange changes as git 
patches (or even via pull/push). In the real world, they send new
versions which I integrate (after dealing with their whitespace and encoding changes...).
Therefore, being able to say 
"git commit --author=mickey"
and having git translate "mickey" into "Mickey Mouse [off-list ref]"
is a real time saver. The patch accomplishes this by reading config keys "user.mickey.name" and "user.mickey.email" when encountering an 
--author argument without "<>".

If there's interest in this patch I'll follow up with a documentation patch.

The "--committer" argument to git commit is not treated because I don't
consider it worthwhile.

Note that the implementation is different from git-svn's author file on
purpose because it serves a different purpose.

Michael

P.S.: That's my first patch here. Yes, I've read Doc/SubmittingPatches.
So, if something's wrong, please be gentle but not overly so ;) 

 builtin-commit.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 649c8be..d90e2f4 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -53,6 +53,12 @@ static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty;
 static char *untracked_files_arg;
+struct user {
+	char *name, *full_name, *email;
+};
+static struct user **users;
+static int users_alloc;
+static int users_nr;
 /*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing

@@ -406,6 +412,7 @@ static const char sign_off_header[] = "Signed-off-by: ";
 static void determine_author_info(void)
 {
 	char *name, *email, *date;
+	int i;
 
 	name = getenv("GIT_AUTHOR_NAME");
 	email = getenv("GIT_AUTHOR_EMAIL");

@@ -429,10 +436,22 @@ static void determine_author_info(void)
 		date = xstrndup(rb + 2, eol - (rb + 2));
 	}
 
+	author_date = date;
+
 	if (force_author) {
 		const char *lb = strstr(force_author, " <");
 		const char *rb = strchr(force_author, '>');
 
+		if (!lb && !rb) {
+			for (i=0; i < users_nr; i++) {
+				if (!strcmp(force_author, users[i]->name)) {
+					author_name = users[i]->full_name;
+					author_email = users[i]->email;
+					return;
+				}
+			}
+		}
+
 		if (!lb || !rb)
 			die("malformed --author parameter");
 		name = xstrndup(force_author, lb - force_author);

@@ -441,7 +460,6 @@ static void determine_author_info(void)
 
 	author_name = name;
 	author_email = email;
-	author_date = date;
 }
 
 static int prepare_to_commit(const char *index_file, const char *prefix)

@@ -888,11 +906,56 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 	}
 }
 
+static struct user *make_user(const char *name, int len)
+{
+	struct user *ret;
+	int i;
+
+	for (i = 0; i < users_nr; i++) {
+		if (len ? (!strncmp(name, users[i]->name, len) &&
+			   !users[i]->name[len]) :
+		    !strcmp(name, users[i]->name))
+			return users[i];
+	}
+
+	ALLOC_GROW(users, users_nr + 1, users_alloc);
+	ret = xcalloc(1, sizeof(struct user));
+	users[users_nr++] = ret;
+	if (len)
+		ret->name = xstrndup(name, len);
+	else
+		ret->name = xstrdup(name);
+
+	return ret;
+}
+
 static int git_commit_config(const char *k, const char *v, void *cb)
 {
+	const char *name;
+	const char *subkey;
+	struct user *user;
+
 	if (!strcmp(k, "commit.template"))
 		return git_config_string(&template_file, k, v);
 
+	if (!prefixcmp(k, "user.")) {
+		name = k + 5;
+		subkey = strrchr(name, '.');
+		if (!subkey)
+			return 0;
+		user = make_user(name, subkey - name);
+		if (!strcmp(subkey, ".name")) {
+			if (!v)
+				return config_error_nonbool(k);
+			user->full_name = xstrdup(v);
+		} else if (!strcmp(subkey, ".email")) {
+			if (!v)
+				return config_error_nonbool(k);
+			user->email = xstrdup(v);
+		}
+		return 0;
+	}
+
 	return git_status_config(k, v, cb);
 }
 

-- 
1.6.0

On Thu, Aug 21, 2008 at 11:19:41AM +0200, Michael J Gruber [off-list ref] wrote:

If there's interest in this patch I'll follow up with a documentation patch.

See http://article.gmane.org/gmane.comp.version-control.git/92913.

Miklos Vajna venit, vidit, dixit 21.08.2008 15:49:

On Thu, Aug 21, 2008 at 11:19:41AM +0200, Michael J Gruber [off-list ref] wrote:

quoted

If there's interest in this patch I'll follow up with a documentation patch.

See http://article.gmane.org/gmane.comp.version-control.git/92913.

I've read the post you quote but I'm not sure you've read that AND my
post. I clearly described/documented what my patch does and why I think
it's useful, in the way it's often done here: after the commit message
and before the diffstat. It is documented (as required in the post you
cite), it just doesn't contain a documentation patch.

Documentation/SubmittingPatches in all its length doesn't contain the
requirement you're reading into Junio's post. Maybe it should, if that's
what is meant.

Michael

Michael J Gruber, Thu, Aug 21, 2008 11:19:41 +0200:

This allows the use of author abbreviations when specifying commit
authors via the --author option to git commit. "--author=$key" is
resolved by looking up "user.$key.name" and "user.$key.email" in the
config.

Isn't there existing well-known formats for mail aliases?
For instance, Mutt uses simple text file:

    alias nickname1 Author Name <mail@address>
    alias nickname2 "Author Name 2" <mail2@address>

I don't know how well-known this is, but is surely more known than
git's config (and there are aliases in that format already).
Maybe just reference such files in git's config?

Alex Riesen, Thu, Aug 21, 2008 19:41:18 +0200:

Michael J Gruber, Thu, Aug 21, 2008 11:19:41 +0200:

quoted

This allows the use of author abbreviations when specifying commit
authors via the --author option to git commit. "--author=$key" is
resolved by looking up "user.$key.name" and "user.$key.email" in the
config.

Isn't there existing well-known formats for mail aliases?
For instance, Mutt uses simple text file:

    alias nickname1 Author Name <mail@address>
    alias nickname2 "Author Name 2" <mail2@address>

I don't know how well-known this is, but is surely more known than
git's config (and there are aliases in that format already).
Maybe just reference such files in git's config?

Oh, and you may consider using .mailmap files (look into the Git's
one for example): the user part of mail address is very often a good
alias (and sometimes famous nickname) of a person: junio, tytso,
davem, alan, viro, hpa... You'll have to define some rules for
duplications, of course (first wins seems to be popular).

On Thu, Aug 21, 2008 at 11:19:41AM +0200, Michael J Gruber wrote:

This allows the use of author abbreviations when specifying commit
authors via the --author option to git commit. "--author=$key" is
resolved by looking up "user.$key.name" and "user.$key.email" in the
config.

This seems like a reasonable feature to me, though two high-level
questions:

  - Is it worth supporting external alias sources, as Alex mentioned? I
    think that would make more sense for many people. Even if you are
    not personally interested in writing it, it would be nice to keep it
    in mind as a future expansion when doing this work. For example,
    maybe it makes more sense for the config to point to a (type, file)
    pair instead of placing directly into the config. Or maybe this
    should just live in conjunction with that feature, if somebody cares
    to implement it.

  - Is user.$key the right namespace? It precludes a few particular
    aliases, and it might clash with future user.* config. Perhaps
    user.alias.* would be a better place (or, as above, just referencing
    an external file).

  - git-send-email already looks at some alias files. Maybe this is an
    opportunity to refactor and centralize (although perhaps it is not
    worth the effort, because of the different implementation
    languages).

---
In an ideal word, all my collaborators would exchange changes as git 
patches (or even via pull/push). In the real world, they send new
versions which I integrate (after dealing with their whitespace and encoding changes...).
Therefore, being able to say 
"git commit --author=mickey"
and having git translate "mickey" into "Mickey Mouse [off-list ref]"
is a real time saver. The patch accomplishes this by reading config keys "user.mickey.name" and "user.mickey.email" when encountering an 
--author argument without "<>".

This justification should probably go into the commit message, not the
cover letter. When you are writing it, think about the reader who will
bisect or blame to your commit a year from now. Will they want to see
just _what_ you did, or _why_ you did it?

If there's interest in this patch I'll follow up with a documentation patch.

I think Miklos already yelled at you for this. The message he referenced
doesn't quite apply, because you did include some discussion of the
"why". The reason I think Junio (and other reviewers) find the "I'll
document this if it is accepted" so frustrating is that it puts them in
an awkward position.

When reviewing, you are trying to say "is this patch OK?". And clearly
it isn't, because it lacks documentation. Now Junio could queue your
patch and wait for the documentation, but sometimes the followup doc
patches aren't as easily forthcoming, and then he has to deal with it
later.

Furthermore, it is sort of a good faith effort. It shows that you put
the work into cleaning up the patch for presenting to the community,
which encourages the community to take a look. Saying "this is half of
the work, and I will do the other half if you like this" makes reviewers
wonder how cleaned up and ready the patch is.

All of that being said, I think in this instance it is less about the
patch and more in the words you picked. If you said "I am thinking about
this feature, and here is how I think the interface should work, and
here is the patch I have so far. I don't want to document the interface
until it is settled, so please comment on that and I will work up a
final patch" then that would have gone over very well. But as it
happens, you chose the magic pet peeve words. ;)

The "--committer" argument to git commit is not treated because I don't
consider it worthwhile.

If you are introducing a new source of alias mappings, it would make
sense to me to support it everywhere for the sake of consistency. That
means --committer should look at it, too (and should only be a few
lines, I would think), and probably git-send-email.

P.S.: That's my first patch here. Yes, I've read Doc/SubmittingPatches.
So, if something's wrong, please be gentle but not overly so ;)

I hope this is the right amount of gentleness. ;)

Others may disagree, but style-wise I think we usually put each struct
member on its own line.

 	if (force_author) {
 		const char *lb = strstr(force_author, " <");
 		const char *rb = strchr(force_author, '>');
 
+		if (!lb && !rb) {
+			for (i=0; i < users_nr; i++) {

Style: "i = 0"

+				if (!strcmp(force_author, users[i]->name)) {
+					author_name = users[i]->full_name;
+					author_email = users[i]->email;
+					return;
+				}

I haven't traced all of the uses of author_name and author_email, but
all of the other codepaths seem to allocate a new string, whereas this
uses the existing strings. Is this going to accidentally free() from the
users list, or are we just leaking those other strings now?

+	ALLOC_GROW(users, users_nr + 1, users_alloc);

Yay, a first-time submitter bothered to use ALLOC_GROW! :)

+	ret = xcalloc(1, sizeof(struct user));
+	users[users_nr++] = ret;
+	if (len)
+		ret->name = xstrndup(name, len);
+	else
+		ret->name = xstrdup(name);
+
+	return ret;
+}

This is the not the most git-ish way of using the config[1]. Usually we
avoid reading big lists into memory, but rather just call git_config
with the appropriate callback when we find we need to look up the user
alias.

[1] However, I don't necessarily agree with this. We can end up parsing
the config (which may be split across 3 files) several times per
command, so it is probably better to just parse and store it in one go.
So I will let Junio comment on the preferred method.

-Peff

First of all: Thanks for all your responses. I think I've learned a lot
through them, and hopefully I'll be able to give evidence with an
upcoming patch...

Jeff King venit, vidit, dixit 21.08.2008 22:02:

On Thu, Aug 21, 2008 at 11:19:41AM +0200, Michael J Gruber wrote:

quoted

This allows the use of author abbreviations when specifying commit 
authors via the --author option to git commit. "--author=$key" is 
resolved by looking up "user.$key.name" and "user.$key.email" in
the config.

This seems like a reasonable feature to me, though two high-level 
questions:

- Is it worth supporting external alias sources, as Alex mentioned? I
 think that would make more sense for many people. Even if you are 
not personally interested in writing it, it would be nice to keep it 
in mind as a future expansion when doing this work. For example, 
maybe it makes more sense for the config to point to a (type, file) 
pair instead of placing directly into the config. Or maybe this 
should just live in conjunction with that feature, if somebody cares 
to implement it.

- Is user.$key the right namespace? It precludes a few particular 
aliases, and it might clash with future user.* config. Perhaps 
user.alias.* would be a better place (or, as above, just referencing 
an external file).

- git-send-email already looks at some alias files. Maybe this is an 
opportunity to refactor and centralize (although perhaps it is not 
worth the effort, because of the different implementation languages).

There's also git svn.
I think all of these serve different purposes, and have different
typical numbers of entries.

- mailmap maps email addresses to full names, for display purposes only.
Typically a long list.

- git svn's author file maps usernames to fullname <email>. But for
every svn repo I need a file following their chosen keys (usernames),
rather than abbreviations I would remember.

- alias files for send-email map keys to fullname <email>. That indeed
is a mapping and a purpose similar to my intention for git commit
--author. Problem here is that it's in perl and supports various
different formats.

I think for send-email you would typically use your mua's alias file.

For git commit --author abbreviations at least I would typically need
only very few entries (be it per repo or globally), which means they can
be much shorter (than my mua aliases) in order to be unique, and I don't
really want an extra file for that.

So, while in fact I wouldn't have been able to implement it differently
anyways, there are other good reasons as well. :)

quoted

--- In an ideal word, all my collaborators would exchange changes

as git patches (or even via pull/push). In the real world, they
send new versions which I integrate (after dealing with their
whitespace and encoding changes...). Therefore, being able to say 
"git commit --author=mickey" and having git translate "mickey" into
"Mickey Mouse [off-list ref]" is a real time saver. The patch
accomplishes this by reading config keys "user.mickey.name" and
"user.mickey.email" when encountering an --author argument without
"<>".

This justification should probably go into the commit message, not
the cover letter. When you are writing it, think about the reader who
will bisect or blame to your commit a year from now. Will they want
to see just _what_ you did, or _why_ you did it?

OK. I think I'm still thinking in terms of "change log style" commit
messages. I haven't completely switched from svn to git yet, neither
technically nor intellectually, it seems. Read "brain rotten" ;)

quoted

If there's interest in this patch I'll follow up with a
documentation patch.

I think Miklos already yelled at you for this. The message he
referenced doesn't quite apply, because you did include some
discussion of the "why".

He didn't mean to yell, we corresponded off-list, all is well.

[snip]

final patch" then that would have gone over very well. But as it 
happens, you chose the magic pet peeve words. ;)

Newcomer's luck, I'm fine with that.

quoted

The "--committer" argument to git commit is not treated because I
don't consider it worthwhile.

I managed to fool everyone, including myself. There is no --committer
option. I feel in good company now ;)

There is GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL, and likewise for
author. My patch does not use any of these, it only deals with (the)
option argument(s). Explicitely set *_{NAME,EMAIL} should be respected
as is.

If you are introducing a new source of alias mappings, it would make 
sense to me to support it everywhere for the sake of consistency.
That means --committer should look at it, too (and should only be a
few lines, I would think), and probably git-send-email.

quoted

P.S.: That's my first patch here. Yes, I've read
Doc/SubmittingPatches. So, if something's wrong, please be gentle
but not overly so ;)

I hope this is the right amount of gentleness. ;)

quoted

--- a/builtin-commit.c +++ b/builtin-commit.c @@ -53,6 +53,12 @@

static char *author_name, *author_email, *author_date; static int
all, edit_flag, also, interactive, only, amend, signoff; static int
quiet, verbose, no_verify, allow_empty; static char
*untracked_files_arg; +struct user { +	char *name, *full_name,
*email; +};

Others may disagree, but style-wise I think we usually put each
struct member on its own line.

quoted

if (force_author) { const char *lb = strstr(force_author, " <"); 
const char *rb = strchr(force_author, '>');

+		if (!lb && !rb) { +			for (i=0; i < users_nr; i++) {

Style: "i = 0"

quoted

+				if (!strcmp(force_author, users[i]->name)) { +					author_name
= users[i]->full_name; +					author_email = users[i]->email; +
return; +				}

I haven't traced all of the uses of author_name and author_email, but
 all of the other codepaths seem to allocate a new string, whereas

..because they need to make a local (for the function) string global
(for the file)...

this uses the existing strings.

...because they are (file) global already.

Is this going to accidentally free()
from the users list, or are we just leaking those other strings now?

Same as branches in remote.c, see below. They're not freed accidentally
in builtin-commit.c

quoted

+	ALLOC_GROW(users, users_nr + 1, users_alloc);

Yay, a first-time submitter bothered to use ALLOC_GROW! :)

quoted

+	ret = xcalloc(1, sizeof(struct user)); +	users[users_nr++] = ret;
 +	if (len) +		ret->name = xstrndup(name, len); +	else +		ret->name
= xstrdup(name); + +	return ret; +}

This is the not the most git-ish way of using the config[1]. Usually
we avoid reading big lists into memory, but rather just call
git_config with the appropriate callback when we find we need to look
up the user alias.

[1] However, I don't necessarily agree with this. We can end up
parsing the config (which may be split across 3 files) several times
per command, so it is probably better to just parse and store it in
one go. So I will let Junio comment on the preferred method.

I was looking all over the existing code for a function which would do
what "git config --get $key" does, and didn't find any. I ended up
copying the logic (and code) from remote.c's parsing of "branch.*.*".
[Should I have attributed this somehow? ]

I understand there are good reasons for this (the way the config is
parsed): a generic central config parser wouldn't be able to verify the
entries when reading the config.
OTOH, verifying an entry when using it wouldn't be that much later. So,
reading the complete config once and storing it in a global struct
should be an alternative which would provide a central place for all
parsing. Judging the implications is way above my current understanding
of the codebase, not to mention implementing it.

Cheers
Michael

P.S.: I should have split this up. Next post will be shorter.

[oops, this accidentally got taken off the list, so here is a repost to
the list and all interested parties]

On Fri, Aug 22, 2008 at 10:27:24AM +0200, Michael J Gruber wrote:

There's also git svn.
I think all of these serve different purposes, and have different
typical numbers of entries.

- mailmap maps email addresses to full names, for display purposes only.
Typically a long list.

- git svn's author file maps usernames to fullname <email>. But for
every svn repo I need a file following their chosen keys (usernames),
rather than abbreviations I would remember.

- alias files for send-email map keys to fullname <email>. That indeed
is a mapping and a purpose similar to my intention for git commit
--author. Problem here is that it's in perl and supports various
different formats.

I agree with your analysis here. The mapping done by mailmap and git-svn
aren't the same. The ones for send-email are, but there is simply an
implementation hurdle.

I think for send-email you would typically use your mua's alias file.

For git commit --author abbreviations at least I would typically need
only very few entries (be it per repo or globally), which means they can
be much shorter (than my mua aliases) in order to be unique, and I don't
really want an extra file for that.

I think this depends on your situation. In your case, it sounds like you
want to configure a few names that frequently have --author fields for
your specific workflow. For me, even though only 1% of the people in my
mua's alias file might send me patches, 99% of the people I would want
to use --author on are in my mua's alias file.

So while there are may only be a few needed entries, they are already
there for me. Of course, I don't really use --author much, since most
people I talk to are already git users. ;) So I am extrapolating a bit.

quoted

quoted

The "--committer" argument to git commit is not treated because I
don't consider it worthwhile.

I managed to fool everyone, including myself. There is no --committer
option. I feel in good company now ;)

Heh.

There is GIT_COMMITTER_NAME and GIT_COMMITTER_EMAIL, and likewise for
author. My patch does not use any of these, it only deals with (the)
option argument(s). Explicitely set *_{NAME,EMAIL} should be respected
as is.

I think that is sensible.

quoted

I haven't traced all of the uses of author_name and author_email, but
 all of the other codepaths seem to allocate a new string, whereas

..because they need to make a local (for the function) string global
(for the file)...

quoted

this uses the existing strings.

...because they are (file) global already.

quoted

Is this going to accidentally free()
from the users list, or are we just leaking those other strings now?

Same as branches in remote.c, see below. They're not freed accidentally
in builtin-commit.c

OK, I see. I wonder if it is worth xstrdup'ing them _anyway_, so that
determine_author_info produces a consistent result, and the person who
later does the free() cleanup won't get a nasty surprise. But the
leakage is probably not enough to really care about in this instance.

I was looking all over the existing code for a function which would do
what "git config --get $key" does, and didn't find any. I ended up
copying the logic (and code) from remote.c's parsing of "branch.*.*".
[Should I have attributed this somehow? ]

No, no need to attribute in this case, I think.

I think the way you have done the config is fine, unless somebody else
has a major style objection (and yes, there are examples of similar
styles).

-Peff

Jeff King [off-list ref] writes:

quoted

For git commit --author abbreviations at least I would typically need
only very few entries (be it per repo or globally), which means they can
be much shorter (than my mua aliases) in order to be unique, and I don't
really want an extra file for that.

I think this depends on your situation. In your case, it sounds like you
want to configure a few names that frequently have --author fields for
your specific workflow. For me, even though only 1% of the people in my
mua's alias file might send me patches, 99% of the people I would want
to use --author on are in my mua's alias file.

So while there are may only be a few needed entries, they are already
there for me. Of course, I don't really use --author much, since most
people I talk to are already git users. ;) So I am extrapolating a bit.

Another potential source of this information is the existing commits.  If
you are communicating with the same set of people already, you already
have the information in your repository.  I suspect Michael's "selected
few co-workers that would comfortably fit in a small list of config
entries without need for any external text file" use case would be better
served by an approach to look into existing commits.

I often use "git who Jeff" alias to fill the recipient of my e-mails with
this alias:

    [alias]
        who = "!sh -c 'git log -1 --pretty=\"format:%an <%ae>\" --author=\"$1\"' -"
        one = "!sh -c 'git show -s --pretty=\"format:%h (%s, %ai\" \"$@\" | sed -e \"s/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9][0-9][0-9]$/)/\"' -"

On Fri, Aug 22, 2008 at 02:09:35PM -0700, Junio C Hamano wrote:

I often use "git who Jeff" alias to fill the recipient of my e-mails with
this alias:

    [alias]
        who = "!sh -c 'git log -1 --pretty=\"format:%an <%ae>\" --author=\"$1\"' -"

Very clever, I like it. And it also solves the problem I sometimes _do_
have, which is pulling aliases into my mua from git.

-Peff

Hi,

On Aug 22, 2008, at 10:09 PM, Junio C Hamano wrote:

Another potential source of this information is the existing  
commits.  If
you are communicating with the same set of people already, you already
have the information in your repository.  I suspect Michael's  
"selected
few co-workers that would comfortably fit in a small list of config
entries without need for any external text file" use case would be  
better
served by an approach to look into existing commits.

I often use "git who Jeff" alias to fill the recipient of my e- 
mails with
this alias:

    [alias]
        who = "!sh -c 'git log -1 --pretty=\"format:%an <%ae>\" -- 
author=\"$1\"' -"

Nice:)

        one = "!sh -c 'git show -s --pretty=\"format:%h (%s, %ai\"  
\"$@\" | sed -e \"s/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9] 
[0-9][0-9]$/)/\"' -"

Can you explain this one? It seems a bit like git describe, but it  
misses a single char at the beggining?

git (master) $ git one
2ebc02d (Start 1.6.1 cycle, 2008-08-17)

git (master) $ git describe
v1.6.0-2-g2ebc02d

Best regards,
-- 
Pedro Melo
Blog: http://www.simplicidade.org/notes/
XMPP ID: melo@simplicidade.org
Use XMPP!

On Sun, Aug 24, 2008 at 10:19:20AM +0100, Pedro Melo wrote:

quoted

        one = "!sh -c 'git show -s --pretty=\"format:%h (%s, %ai\"  
\"$@\" | sed -e \"s/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9] 
[0-9][0-9]$/)/\"' -"

Can you explain this one? It seems a bit like git describe, but it misses 
a single char at the beggining?

git (master) $ git one
2ebc02d (Start 1.6.1 cycle, 2008-08-17)

git (master) $ git describe
v1.6.0-2-g2ebc02d

The 'g' character is not part of the sha1, but just a prefix used by git
describe. The point of this alias is to refer (in email or other
writing) to commits. Obviously just the sha1 would be sufficient, but
the subject and date of the commit gives the reader some context without
them having to plug it into git-show.

-Peff

On Fri, Aug 22, 2008 at 02:09:35PM -0700, Junio C Hamano wrote:

    [alias]
        who = "!sh -c 'git log -1 --pretty=\"format:%an <%ae>\" --author=\"$1\"' -"

I have two improvements for this, and one of them caused me to find a
git bug, for which the fix is below. :)

  1. I tried this with --no-pager, which made it obvious that this
     should be using --pretty=tformat to append a newline.

  2. I use it with "-i" so I don't have to hit the shift key. And that's
     what revealed the bug.

-- >8 --
fix "git log -i --grep"

This has been broken in v1.6.0 due to the reorganization of
the revision option parsing code. The "-i" is completely
ignored, but works fine in "git log --grep -i".

What happens is that the code for "-i" looks for
revs->grep_filter; if it is NULL, we do nothing, since there
are no grep filters. But that is obviously not correct,
since we want it to influence the later --grep option. Doing
it the other way around works, since "-i" just impacts the
existing grep_filter option.

The fix is to allocate the grep_filter member whenever we
get _any_ grep information, be it actual filters or just
flags. Thus checking for non-NULL revs->grep_filter is no
longer sufficient to know that we have patterns; in
commit_match we must actually check that the pattern list is
not empty.

Signed-off-by: Jeff King <redacted>
---
I didn't bother bisecting, but I'm pretty sure this was a fallout from
Pierre's revision option parsing rewrite.

This was generated with -U5 to make the first hunk easier to read.

We could potentially make revs->grep_filter a part of the struct, rather
than malloc'ing it (since we have to look inside grep_filter anyway to
see if there are any patterns). But that still doesn't save us from a
setup_grep call, since we have to initialize some values inside it.
Potentially this setup (which is not very costly) could just be done
when initializing the rev_info struct, and then we could just assume
that grep_filter was always valid.

I went with the less intrusive change in this case, but I am happy to
work it up the other way.

 revision.c     |   25 +++++++++++++++----------
 t/t4202-log.sh |   22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/revision.c b/revision.c
index 8cd39da..a73612f 100644
--- a/revision.c
+++ b/revision.c

@@ -942,19 +942,24 @@ void read_revisions_from_stdin(struct rev_info *revs)
 		if (handle_revision_arg(line, revs, 0, 1))
 			die("bad revision '%s'", line);
 	}
 }
 
-static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
+static void setup_grep(struct rev_info *revs)
 {
 	if (!revs->grep_filter) {
 		struct grep_opt *opt = xcalloc(1, sizeof(*opt));
 		opt->status_only = 1;
 		opt->pattern_tail = &(opt->pattern_list);
 		opt->regflags = REG_NEWLINE;
 		revs->grep_filter = opt;
 	}
+}
+
+static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
+{
+	setup_grep(revs);
 	append_grep_pattern(revs->grep_filter, ptn,
 			    "command line", 0, what);
 }
 
 static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)

@@ -1167,21 +1172,21 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!prefixcmp(arg, "--committer=")) {
 		add_header_grep(revs, "committer", arg+12);
 	} else if (!prefixcmp(arg, "--grep=")) {
 		add_message_grep(revs, arg+7);
 	} else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
-		if (revs->grep_filter)
-			revs->grep_filter->regflags |= REG_EXTENDED;
+		setup_grep(revs);
+		revs->grep_filter->regflags |= REG_EXTENDED;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
-		if (revs->grep_filter)
-			revs->grep_filter->regflags |= REG_ICASE;
+		setup_grep(revs);
+		revs->grep_filter->regflags |= REG_ICASE;
 	} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
-		if (revs->grep_filter)
-			revs->grep_filter->fixed = 1;
+		setup_grep(revs);
+		revs->grep_filter->fixed = 1;
 	} else if (!strcmp(arg, "--all-match")) {
-		if (revs->grep_filter)
-			revs->grep_filter->all_match = 1;
+		setup_grep(revs);
+		revs->grep_filter->all_match = 1;
 	} else if (!prefixcmp(arg, "--encoding=")) {
 		arg += 11;
 		if (strcmp(arg, "none"))
 			git_log_output_encoding = xstrdup(arg);
 		else

@@ -1647,11 +1652,11 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 	return 0;
 }
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
-	if (!opt->grep_filter)
+	if (!opt->grep_filter || !opt->grep_filter->pattern_list)
 		return 1;
 	return grep_buffer(opt->grep_filter,
 			   NULL, /* we say nothing, not even filename */
 			   commit->buffer, strlen(commit->buffer));
 }

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 4c8af45..0ab925c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh

@@ -67,9 +67,31 @@ test_expect_success 'diff-filter=D' '
 		false
 	}
 
 '
 
+test_expect_success 'setup case sensitivity tests' '
+	echo case >one &&
+	test_tick &&
+	git commit -a -m Second
+'
+
+test_expect_success 'log --grep' '
+	echo second >expect &&
+	git log -1 --pretty="tformat:%s" --grep=sec >actual &&
+	test_cmp expect actual
+'
 
+test_expect_success 'log -i --grep' '
+	echo Second >expect &&
+	git log -1 --pretty="tformat:%s" -i --grep=sec >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --grep -i' '
+	echo Second >expect &&
+	git log -1 --pretty="tformat:%s" --grep=sec -i >actual &&
+	test_cmp expect actual
+'
 
 test_done
 

-- 
1.6.0.150.gc3242.dirty

On Sun, Aug 24, 2008 at 09:38:37PM -0400, Jeff King wrote:

This was generated with -U5 to make the first hunk easier to read.

And while doing that, I detected another bug. Or maybe a feature,
depending on your perspective.

-- >8 --
format-patch: use default diff format even with patch options

Previously, running "git format-patch -U5" would cause the
low-level diff machinery to change the diff output format
from "not specified" to "patch". This meant that
format-patch thought we explicitly specified a diff output
format, and would not use the default format. The resulting
message lacked both the diffstat and the summary, as well as
the separating "---".

Now format-patch explicitly checks for this condition and
uses the default. That means that "git format-patch -p" will
now have the "-p" ignored.

Signed-off-by: Jeff King <redacted>
---
Maybe this is intentional, and that by asking for "-U" I am explicitly
saying "I really want the patch format, not the default." But I think
this more reasonably maps to what the user expects.

I am a little uncomfortable hurting anyone who thought that
"format-patch -p" was a good idea. OTOH:

  1. I have to question why they were using format-patch in the first
     place. Probably git-log --pretty=email would be a better fit.

  2. Their mails were already broken, since the presence of the diffstat
     is what triggers the "---" divider.

 builtin-log.c           |    3 ++-
 t/t4014-format-patch.sh |   25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 9204ffd..1d3c5cb 100644
--- a/builtin-log.c
+++ b/builtin-log.c

@@ -932,7 +932,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
-	if (!rev.diffopt.output_format)
+	if (!rev.diffopt.output_format
+		|| rev.diffopt.output_format == DIFF_FORMAT_PATCH)
 		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
 
 	if (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7fe853c..9d99dc2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh

@@ -230,4 +230,29 @@ test_expect_success 'shortlog of cover-letter wraps overly-long onelines' '
 
 '
 
+cat > expect << EOF
+---
+ file |   16 ++++++++++++++++
+ 1 files changed, 16 insertions(+), 0 deletions(-)
+
+diff --git a/file b/file
+index 40f36c6..2dc5c23 100644
+--- a/file
++++ b/file
+@@ -13,4 +13,20 @@ C
+ 10
+ D
+ E
+ F
++5
+EOF
+
+test_expect_success 'format-patch respects -U' '
+
+	git format-patch -U4 -2 &&
+	sed -e "1,/^$/d" -e "/^+5/q" < 0001-This-is-an-excessively-long-subject-line-for-a-messa.patch > output &&
+	test_cmp expect output
+
+'
+
 test_done

-- 
1.6.0.150.gc3242.dirty

This allows the use of author abbreviations when specifying commit
authors via the --author option to git commit. "--author=$key" is
resolved by looking up "user.$key.name" and "user.$key.email" in the
config.

In an ideal word, all my collaborators would exchange changes as git
patches (or even via pull/push). In the real world, they send new
versions which I integrate (after dealing with their whitespace and
encoding changes...). Therefore, being able to say "git commit
--author=mickey" and having git translate "mickey" into "Mickey Mouse
[off-list ref]" is a real time saver. The patch accomplishes
this by reading config keys "user.mickey.name" and "user.mickey.email"
when encountering an --author argument without "<>".

Signed-off-by: Michael J Gruber <redacted>
---

I tried to apply everything I've learned from this thread:
- Justification in commit message rather than cover
- minor style adjustments
- xstrdup two more strings to spare future leakage cleanup-a-thons a few
  unpleasant surprises
- comes with documentation patch now

I think the relation to and distinction from "git-svn -A" and ".mailmap"
has become clear through the discussion (should a summary go in the commit
message?).
I really like Junio's alias (git who). It's certainly helpful. For the
case of "git commit --author key" I think we should not simply go by the
first, possibly non-unique match returned by "git show". Also, being able
to say "git commit --author=nitpicker" may make some days brighter ;)

 Documentation/config.txt     |    8 +++++
 Documentation/git-commit.txt |    5 ++-
 builtin-commit.c             |   67 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9020675..9bea3a3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1107,6 +1107,14 @@ user.signingkey::
 	unchanged to gpg's --local-user parameter, so you may specify a key
 	using any method that gpg supports.
 
+user.<author>.email::
+	The email address to be recorded in a newly created commit if you
+	specify the option \--author=<author> to linkgit:git-commit[1].
+
+user.<author>.name::
+	The full name to be recorded in a newly created commit if you
+	specify the option \--author=<author> to linkgit:git-commit[1].
+
 imap::
 	The configuration variables in the 'imap' section are described
 	in linkgit:git-imap-send[1].

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0e25bb8..1685cf6 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt

@@ -76,7 +76,10 @@ OPTIONS
 
 --author=<author>::
 	Override the author name used in the commit.  Use
-	`A U Thor <author@example.com>` format.
+	`A U Thor <author@example.com>` format. Alternatively, if
+	<author> does not contain `<>` then the configuration
+	variables `user.<author>.name` and `user.<author>.email`
+	are used if present (see linkgit:git-config[1]).
 
 -m <msg>::
 --message=<msg>::

diff --git a/builtin-commit.c b/builtin-commit.c
index 649c8be..c36e60f 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -53,6 +53,14 @@ static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty;
 static char *untracked_files_arg;
+struct user {
+	char *name;
+	char *full_name;
+	char *email;
+};
+static struct user **users;
+static int users_alloc;
+static int users_nr;
 /*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing

@@ -406,6 +414,7 @@ static const char sign_off_header[] = "Signed-off-by: ";
 static void determine_author_info(void)
 {
 	char *name, *email, *date;
+	int i;
 
 	name = getenv("GIT_AUTHOR_NAME");
 	email = getenv("GIT_AUTHOR_EMAIL");

@@ -429,10 +438,22 @@ static void determine_author_info(void)
 		date = xstrndup(rb + 2, eol - (rb + 2));
 	}
 
+	author_date = date;
+
 	if (force_author) {
 		const char *lb = strstr(force_author, " <");
 		const char *rb = strchr(force_author, '>');
 
+		if (!lb && !rb) {
+			for (i = 0; i < users_nr; i++) {
+				if (!strcmp(force_author, users[i]->name)) {
+					author_name = xstrdup(users[i]->full_name);
+					author_email = xstrdup(users[i]->email);
+					return;
+				}
+			}
+		}
+
 		if (!lb || !rb)
 			die("malformed --author parameter");
 		name = xstrndup(force_author, lb - force_author);

@@ -441,7 +462,6 @@ static void determine_author_info(void)
 
 	author_name = name;
 	author_email = email;
-	author_date = date;
 }
 
 static int prepare_to_commit(const char *index_file, const char *prefix)

@@ -888,11 +908,56 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 	}
 }
 
+static struct user *make_user(const char *name, int len)
+{
+	struct user *ret;
+	int i;
+
+	for (i = 0; i < users_nr; i++) {
+		if (len ? (!strncmp(name, users[i]->name, len) &&
+			   !users[i]->name[len]) :
+		    !strcmp(name, users[i]->name))
+			return users[i];
+	}
+
+	ALLOC_GROW(users, users_nr + 1, users_alloc);
+	ret = xcalloc(1, sizeof(struct user));
+	users[users_nr++] = ret;
+	if (len)
+		ret->name = xstrndup(name, len);
+	else
+		ret->name = xstrdup(name);
+
+	return ret;
+}
+
 static int git_commit_config(const char *k, const char *v, void *cb)
 {
+	const char *name;
+	const char *subkey;
+	struct user *user;
+
 	if (!strcmp(k, "commit.template"))
 		return git_config_string(&template_file, k, v);
 
+	if (!prefixcmp(k, "user.")) {
+		name = k + 5;
+		subkey = strrchr(name, '.');
+		if (!subkey)
+			return 0;
+		user = make_user(name, subkey - name);
+		if (!strcmp(subkey, ".name")) {
+			if (!v)
+				return config_error_nonbool(k);
+			user->full_name = xstrdup(v);
+		} else if (!strcmp(subkey, ".email")) {
+			if (!v)
+				return config_error_nonbool(k);
+			user->email = xstrdup(v);
+		}
+		return 0;
+	}
+
 	return git_status_config(k, v, cb);
 }
 

-- 
1.6.0