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

From: Jeff King <peff@peff.net>
Date: Aug 22, 2008, 16:50 UTC
Message-ID: <20080822165047.GA3339@sigill.intra.peff.net>
In-Reply-To: <48AE786C.20201@fastmail.fm>

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

Show 15 quoted lines

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

Show 6 quoted lines

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

Show 5 quoted lines

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

Show 15 quoted lines

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

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