[PATCH] Add support for git's mailmap.

[PATCH] Add support for git's mailmap.

[smithj4]

If a mailmap file is present in the repo, it will be used to coalesce
commits by the same person, just like git does. When no mailmap file is
found then it functions as before.
---
  cgit.h      |  3 +++
  parsing.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
  ui-atom.c   |  9 +++++++--
  ui-commit.c |  4 ++++
  ui-log.c    |  4 ++++
  ui-stats.c  |  8 ++++++--
  6 files changed, 67 insertions(+), 4 deletions(-)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-support-for-git-s-mailmap.patch
Type: text/x-patch
Size: 7405 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160824/27e9792a/attachment.bin>

[PATCH] Add support for git's mailmap.

[john]

On Wed, Aug 24, 2016 at 02:28:16PM -0400, Jason A. Smith wrote:
> 
> If a mailmap file is present in the repo, it will be used to coalesce
> commits by the same person, just like git does. When no mailmap file is
> found then it functions as before.

Missing sign-off, see [1] for what this means.

[1] http://developercertificate.org/

> ---
>   cgit.h      |  3 +++
>   parsing.c   | 43 +++++++++++++++++++++++++++++++++++++++++++
>   ui-atom.c   |  9 +++++++--
>   ui-commit.c |  4 ++++
>   ui-log.c    |  4 ++++
>   ui-stats.c  |  8 ++++++--
>   6 files changed, 67 insertions(+), 4 deletions(-)
> 
> diff --git a/cgit.h b/cgit.h
> index 325432b..737ab92 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -24,6 +24,7 @@
>  #include <utf8.h>
>  #include <notes.h>
>  #include <graph.h>
> +#include <mailmap.h>
>  
>  /* Add isgraph(x) to Git's sane ctype support (see git-compat-util.h) */
>  #undef isgraph
> @@ -370,6 +371,8 @@ extern char *fmtalloc(const char *format,...);
>  extern struct commitinfo *cgit_parse_commit(struct commit *commit);
>  extern struct taginfo *cgit_parse_tag(struct tag *tag);
>  extern void cgit_parse_url(const char *url);
> +extern int cgit_read_mailmap(struct string_list *map);
> +extern int cgit_map_user(struct string_list *map, char **email, char **name);
>  
>  extern const char *cgit_repobasename(const char *reponame);
>  
> diff --git a/parsing.c b/parsing.c
> index 9dacb16..7e7edd1 100644
> --- a/parsing.c
> +++ b/parsing.c
> @@ -88,6 +88,49 @@ static void parse_user(const char *t, char **name, char **email, unsigned long *
>  	}
>  }
>  
> +/* Cgit surrounds the email address with angle brackets while git does not.
> +   Copy the email address, remove the angle brackets, perform the mailmap
> +   user lookup and finally replace the name and email address with the new
> +   values, and put the angle brackets back around the email. */

/*
 * None of the other multiline comments in this code base
 * look like the one above :-)
 */

> +int cgit_read_mailmap(struct string_list *map)
> +{
> +	int err = 0;
> +
> +	memset(map, 0, sizeof(*map));
> +	err = read_mailmap(map, NULL);
> +	return err;
> +}

Missing blank line here, but I'm not sure why the function above exists.
Other than the memset(), it doesn't do anything except call
read_mailmap() and the memset is definitely wrong, it leaks memory if
string_list is non-empty and if you're worried that map might be
uninitialized then this should be calling string_list_init(), but I'd
argue that it's up to the call site and since you always allocate on the
stack you can just assign STRING_LIST_INIT_NODUP on declaration.

> +int cgit_map_user(struct string_list *map, char **email, char **name)
> +{
> +	char *map_email, *map_name;
> +	size_t emaillen, namelen;
> +	int ret;
> +
> +	emaillen = strlen(*email);
> +	namelen = strlen(*name);
> +	map_email = xmalloc(emaillen + 1);
> +	map_name = xmalloc(namelen + 1);
> +	strncpy(map_email, *email, emaillen + 1);
> +	strncpy(map_name, *name, namelen + 1);
> +	memmove(map_email, map_email+1, emaillen-2);
> +	map_email[emaillen-2] = '\0';
> +	emaillen = emaillen - 2;
> +	ret = map_user(map, (const char**)&map_email, &emaillen,
> +			    (const char**)&map_name, &namelen);
> +	if (ret) {
> +		*email = xmalloc(strlen("<") + emaillen + strlen(">") + 1);
> +		sprintf(*email, "<%.*s>", (int)emaillen, map_email);
> +		*name = xmalloc(namelen + 1);
> +		strncpy(*name, map_name, namelen + 1);
> +	} else {
> +		/* When a mapping is found they are pointing to the
> +		   actual mailmap list entry and cannot be freed. */
> +
> +		free(map_email);
> +		free(map_name);
> +	}
> +
> +	return ret;
> +}

I think we can do a log better than this if we step back and think about
the call sites below.  There's no need to modify the author and
committer fields in struct commitinfo if we first refactor to introduce
local variables:

	const char *author = info->author;
	const char *author_email = info->author_email;

Then we can almost use map_user() directly if not for the pesky angle
brackets, but looking at the use sites, ui-atom.c is already prepared
for the email address not to have them and there are a few HTML sites
that are trivial to change.  The only difficult case is the email filter
and we could create a new cgit_print_ident() function to wrap all of
that up nicely.

It's a bigger change and will turn this single patch into a series, but
the end result will be much cleaner.

>  #ifdef NO_ICONV
>  #define reencode(a, b, c)
>  #else
> diff --git a/ui-atom.c b/ui-atom.c
> index 41838d3..e0d1144 100644
> --- a/ui-atom.c
> +++ b/ui-atom.c
> @@ -11,7 +11,8 @@
>  #include "html.h"
>  #include "ui-shared.h"
>  
> -static void add_entry(struct commit *commit, const char *host)
> +static void add_entry(struct commit *commit, const char *host,
> +		      struct string_list *mailmap)
>  {
>  	char delim = '&';
>  	char *hex;
> @@ -19,6 +20,7 @@ static void add_entry(struct commit *commit, const char *host)
>  	struct commitinfo *info;
>  
>  	info = cgit_parse_commit(commit);
> +	cgit_map_user(mailmap, &(info->author_email), &(info->author));
>  	hex = oid_to_hex(&commit->object.oid);
>  	html("<entry>\n");
>  	html("<title>");
> @@ -89,6 +91,7 @@ void cgit_print_atom(char *tip, char *path, int max_count)
>  	const char *argv[] = {NULL, tip, NULL, NULL, NULL};
>  	struct commit *commit;
>  	struct rev_info rev;
> +	struct string_list mailmap;
>  	int argc = 2;
>  
>  	if (ctx.qry.show_all)
> @@ -108,6 +111,8 @@ void cgit_print_atom(char *tip, char *path, int max_count)
>  	rev.show_root_diff = 0;
>  	rev.max_count = max_count;
>  	setup_revisions(argc, argv, &rev, NULL);
> +	cgit_read_mailmap(&mailmap);
> +	rev.mailmap = &mailmap;
>  	prepare_revision_walk(&rev);
>  
>  	host = cgit_hosturl();
> @@ -139,7 +144,7 @@ void cgit_print_atom(char *tip, char *path, int max_count)
>  		free(repourl);
>  	}
>  	while ((commit = get_revision(&rev)) != NULL) {
> -		add_entry(commit, host);
> +		add_entry(commit, host, &mailmap);
>  		free_commit_buffer(commit);
>  		free_commit_list(commit->parents);
>  		commit->parents = NULL;
> diff --git a/ui-commit.c b/ui-commit.c
> index 099d294..27d5931 100644
> --- a/ui-commit.c
> +++ b/ui-commit.c
> @@ -18,6 +18,7 @@ void cgit_print_commit(char *hex, const char *prefix)
>  	struct commit *commit, *parent;
>  	struct commitinfo *info, *parent_info;
>  	struct commit_list *p;
> +	struct string_list mailmap;
>  	struct strbuf notes = STRBUF_INIT;
>  	unsigned char sha1[20];
>  	char *tmp, *tmp2;
> @@ -38,6 +39,9 @@ void cgit_print_commit(char *hex, const char *prefix)
>  		return;
>  	}
>  	info = cgit_parse_commit(commit);
> +	cgit_read_mailmap(&mailmap);
> +	cgit_map_user(&mailmap, &(info->author_email), &(info->author));
> +	cgit_map_user(&mailmap, &(info->committer_email), &(info->committer));
>  
>  	format_display_notes(sha1, &notes, PAGE_ENCODING, 0);
>  
> diff --git a/ui-log.c b/ui-log.c
> index c97b8e0..90fe5f8 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -238,6 +238,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
>  			 oid_to_hex(&commit->object.oid), ctx.qry.vpath);
>  	show_commit_decorations(commit);
>  	html("</td><td>");
> +	cgit_map_user(revs->mailmap, &(info->author_email), &(info->author));
>  	cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
>  	html_txt(info->author);
>  	cgit_close_filter(ctx.repo->email_filter);
> @@ -360,6 +361,7 @@ static char *next_token(char **src)
>  void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern,
>  		    char *path, int pager, int commit_graph, int commit_sort)
>  {
> +	struct string_list mailmap;
>  	struct rev_info rev;
>  	struct commit *commit;
>  	struct argv_array rev_argv = ARGV_ARRAY_INIT;
> @@ -443,6 +445,8 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern
>  		DIFF_XDL_SET(&rev.diffopt, IGNORE_WHITESPACE);
>  
>  	compile_grep_patterns(&rev.grep_filter);
> +	cgit_read_mailmap(&mailmap);
> +	rev.mailmap = &mailmap;
>  	prepare_revision_walk(&rev);
>  
>  	if (pager) {
> diff --git a/ui-stats.c b/ui-stats.c
> index 7acd358..9ca55f6 100644
> --- a/ui-stats.c
> +++ b/ui-stats.c
> @@ -159,7 +159,7 @@ const char *cgit_find_stats_periodname(int idx)
>  }
>  
>  static void add_commit(struct string_list *authors, struct commit *commit,
> -	const struct cgit_period *period)
> +	const struct cgit_period *period, struct string_list *mailmap)
>  {
>  	struct commitinfo *info;
>  	struct string_list_item *author, *item;
> @@ -171,6 +171,7 @@ static void add_commit(struct string_list *authors, struct commit *commit,
>  	uintptr_t *counter;
>  
>  	info = cgit_parse_commit(commit);
> +	cgit_map_user(mailmap, &(info->author_email), &(info->author));
>  	tmp = xstrdup(info->author);
>  	author = string_list_insert(authors, tmp);
>  	if (!author->util)
> @@ -209,6 +210,7 @@ static int cmp_total_commits(const void *a1, const void *a2)
>  static struct string_list collect_stats(const struct cgit_period *period)
>  {
>  	struct string_list authors;
> +	struct string_list mailmap;
>  	struct rev_info rev;
>  	struct commit *commit;
>  	const char *argv[] = {NULL, ctx.qry.head, NULL, NULL, NULL, NULL};
> @@ -237,10 +239,12 @@ static struct string_list collect_stats(const struct cgit_period *period)
>  	rev.verbose_header = 1;
>  	rev.show_root_diff = 0;
>  	setup_revisions(argc, argv, &rev, NULL);
> +	cgit_read_mailmap(&mailmap);
> +	rev.mailmap = &mailmap;
>  	prepare_revision_walk(&rev);
>  	memset(&authors, 0, sizeof(authors));
>  	while ((commit = get_revision(&rev)) != NULL) {
> -		add_commit(&authors, commit, period);
> +		add_commit(&authors, commit, period, &mailmap);
>  		free_commit_buffer(commit);
>  		free_commit_list(commit->parents);
>  		commit->parents = NULL;
> 

> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit

[PATCH] Add support for git's mailmap.

[smithj4]

I will send a new updated patch that I think will fix most of the minor 
issues, but I am not exactly sure what you mean by the last note.

On 08/24/2016 03:23 PM, John Keeping wrote:
>> +int cgit_map_user(struct string_list *map, char **email, char **name)
>> +{
>> +	char *map_email, *map_name;
>> +	size_t emaillen, namelen;
>> +	int ret;
>> +
>> +	emaillen = strlen(*email);
>> +	namelen = strlen(*name);
>> +	map_email = xmalloc(emaillen + 1);
>> +	map_name = xmalloc(namelen + 1);
>> +	strncpy(map_email, *email, emaillen + 1);
>> +	strncpy(map_name, *name, namelen + 1);
>> +	memmove(map_email, map_email+1, emaillen-2);
>> +	map_email[emaillen-2] = '\0';
>> +	emaillen = emaillen - 2;
>> +	ret = map_user(map, (const char**)&map_email, &emaillen,
>> +			    (const char**)&map_name, &namelen);
>> +	if (ret) {
>> +		*email = xmalloc(strlen("<") + emaillen + strlen(">") + 1);
>> +		sprintf(*email, "<%.*s>", (int)emaillen, map_email);
>> +		*name = xmalloc(namelen + 1);
>> +		strncpy(*name, map_name, namelen + 1);
>> +	} else {
>> +		/* When a mapping is found they are pointing to the
>> +		   actual mailmap list entry and cannot be freed. */
>> +
>> +		free(map_email);
>> +		free(map_name);
>> +	}
>> +
>> +	return ret;
>> +}
>
> I think we can do a log better than this if we step back and think about
> the call sites below.  There's no need to modify the author and
> committer fields in struct commitinfo if we first refactor to introduce
> local variables:
>
> 	const char *author = info->author;
> 	const char *author_email = info->author_email;
>
> Then we can almost use map_user() directly if not for the pesky angle
> brackets, but looking at the use sites, ui-atom.c is already prepared
> for the email address not to have them and there are a few HTML sites
> that are trivial to change.  The only difficult case is the email filter
> and we could create a new cgit_print_ident() function to wrap all of
> that up nicely.
>
> It's a bigger change and will turn this single patch into a series, but
> the end result will be much cleaner.

First a little explanation. I created the wrapper around git's map_user 
for 2 reasons. First, because of the angle brackets that wrap the email 
address in the cgit code. Second, to make copies of the name & email 
address because when map_user finds a match it changes the char pointers 
to point to the appropriate mailmap list entry, and in the call sites, 
at the end of the function, it was freeing the info struct which would 
break subsequent map lookups. This seemed to be the simplest solution, 
to fix both of these problems in one function.

Are you proposing to remove the angle brackets around the email address 
everywhere in cgit strings, to match git, which I think is done in the 
parse_user function? Then use git's map_user directly and create a 
cgit_print_ident function that inserts the angle brackets when printed? 
Then also making local copies of the pointers so freeing the info 
structure doesn't break the mailmap? I think this would be a much larger 
change beyond just supporting the mailmap.

~Jason

[PATCH] Add support for git's mailmap.

[smithj4]

If a mailmap file is present in the repo, it will be used to coalesce
commits by the same person, just like git does. When no mailmap file is
found then it functions as before.

Signed-off-by: Jason A. Smith <smithj4 at bnl.gov>
---
  cgit.h      |  2 ++
  parsing.c   | 39 +++++++++++++++++++++++++++++++++++++++
  ui-atom.c   |  9 +++++++--
  ui-commit.c |  4 ++++
  ui-log.c    |  4 ++++
  ui-stats.c  |  8 ++++++--
  6 files changed, 62 insertions(+), 4 deletions(-)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-support-for-git-s-mailmap.patch
Type: text/x-patch
Size: 7319 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160824/3295cb74/attachment.bin>

[PATCH] Add support for git's mailmap.

[john]

On Wed, Aug 24, 2016 at 05:27:31PM -0400, Jason A. Smith wrote:
> I will send a new updated patch that I think will fix most of the minor 
> issues, but I am not exactly sure what you mean by the last note.
> 
> On 08/24/2016 03:23 PM, John Keeping wrote:
> >> +int cgit_map_user(struct string_list *map, char **email, char **name)
> >> +{
> >> +	char *map_email, *map_name;
> >> +	size_t emaillen, namelen;
> >> +	int ret;
> >> +
> >> +	emaillen = strlen(*email);
> >> +	namelen = strlen(*name);
> >> +	map_email = xmalloc(emaillen + 1);
> >> +	map_name = xmalloc(namelen + 1);
> >> +	strncpy(map_email, *email, emaillen + 1);
> >> +	strncpy(map_name, *name, namelen + 1);
> >> +	memmove(map_email, map_email+1, emaillen-2);
> >> +	map_email[emaillen-2] = '\0';
> >> +	emaillen = emaillen - 2;
> >> +	ret = map_user(map, (const char**)&map_email, &emaillen,
> >> +			    (const char**)&map_name, &namelen);
> >> +	if (ret) {
> >> +		*email = xmalloc(strlen("<") + emaillen + strlen(">") + 1);
> >> +		sprintf(*email, "<%.*s>", (int)emaillen, map_email);
> >> +		*name = xmalloc(namelen + 1);
> >> +		strncpy(*name, map_name, namelen + 1);
> >> +	} else {
> >> +		/* When a mapping is found they are pointing to the
> >> +		   actual mailmap list entry and cannot be freed. */
> >> +
> >> +		free(map_email);
> >> +		free(map_name);
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >
> > I think we can do a log better than this if we step back and think about
> > the call sites below.  There's no need to modify the author and
> > committer fields in struct commitinfo if we first refactor to introduce
> > local variables:
> >
> > 	const char *author = info->author;
> > 	const char *author_email = info->author_email;
> >
> > Then we can almost use map_user() directly if not for the pesky angle
> > brackets, but looking at the use sites, ui-atom.c is already prepared
> > for the email address not to have them and there are a few HTML sites
> > that are trivial to change.  The only difficult case is the email filter
> > and we could create a new cgit_print_ident() function to wrap all of
> > that up nicely.
> >
> > It's a bigger change and will turn this single patch into a series, but
> > the end result will be much cleaner.
> 
> First a little explanation. I created the wrapper around git's map_user 
> for 2 reasons. First, because of the angle brackets that wrap the email 
> address in the cgit code. Second, to make copies of the name & email 
> address because when map_user finds a match it changes the char pointers 
> to point to the appropriate mailmap list entry, and in the call sites, 
> at the end of the function, it was freeing the info struct which would 
> break subsequent map lookups. This seemed to be the simplest solution, 
> to fix both of these problems in one function.
> 
> Are you proposing to remove the angle brackets around the email address 
> everywhere in cgit strings, to match git, which I think is done in the 
> parse_user function? Then use git's map_user directly and create a 
> cgit_print_ident function that inserts the angle brackets when printed? 
> Then also making local copies of the pointers so freeing the info 
> structure doesn't break the mailmap? I think this would be a much larger 
> change beyond just supporting the mailmap.

Yes, pretty much exactly this.  There's only a small number of sites
that actually use author_email and committer_email, most of which are
already printing something, so the fallout shouldn't be too bad.

The only other cases are opening the email filter, which is why I
suggested wrapping at least part of that process in a new function.

[PATCH] Add support for git's mailmap.

[smithj4]

On 08/24/2016 06:30 PM, John Keeping wrote:
> On Wed, Aug 24, 2016 at 05:27:31PM -0400, Jason A. Smith wrote:
>> Are you proposing to remove the angle brackets around the email address
>> everywhere in cgit strings, to match git, which I think is done in the
>> parse_user function? Then use git's map_user directly and create a
>> cgit_print_ident function that inserts the angle brackets when printed?
>> Then also making local copies of the pointers so freeing the info
>> structure doesn't break the mailmap? I think this would be a much larger
>> change beyond just supporting the mailmap.
>
> Yes, pretty much exactly this.  There's only a small number of sites
> that actually use author_email and committer_email, most of which are
> already printing something, so the fallout shouldn't be too bad.
>
> The only other cases are opening the email filter, which is why I
> suggested wrapping at least part of that process in a new function.

So how do you propose splitting the patches to accomplish this? One 
patch to add mailmap support and another patch to modify how angle 
brackets are handled in the email addresses? Which patch goes first and 
who will fix the angle brackets? Are you volunteering? ;)

I haven't done much C programming in years and wasn't familiar with 
neither the git nor cgit code, so it took me a little while to figure 
out how to add mailmap support to cgit. I did it because we recently 
added mailmap files to our local git repos to clean things up and I 
noticed that cgit wasn't using them. I just looked at the filter code a 
little and it looks a bit complicated to my novice eyes. I'm not sure 
how to patch it to just modify only email addresses passed into it. 
Another option might be to just just use another char string to wrap the 
email address in angle brackets before opening the filter in the caller 
and pass that instead?

~Jason

[PATCH] Add support for git's mailmap.

[john]

On Thu, Aug 25, 2016 at 03:22:35PM -0400, Jason A. Smith wrote:
> On 08/24/2016 06:30 PM, John Keeping wrote:
> > On Wed, Aug 24, 2016 at 05:27:31PM -0400, Jason A. Smith wrote:
> >> Are you proposing to remove the angle brackets around the email address
> >> everywhere in cgit strings, to match git, which I think is done in the
> >> parse_user function? Then use git's map_user directly and create a
> >> cgit_print_ident function that inserts the angle brackets when printed?
> >> Then also making local copies of the pointers so freeing the info
> >> structure doesn't break the mailmap? I think this would be a much larger
> >> change beyond just supporting the mailmap.
> >
> > Yes, pretty much exactly this.  There's only a small number of sites
> > that actually use author_email and committer_email, most of which are
> > already printing something, so the fallout shouldn't be too bad.
> >
> > The only other cases are opening the email filter, which is why I
> > suggested wrapping at least part of that process in a new function.
> 
> So how do you propose splitting the patches to accomplish this? One 
> patch to add mailmap support and another patch to modify how angle 
> brackets are handled in the email addresses? Which patch goes first and 
> who will fix the angle brackets? Are you volunteering? ;)

The cleanup goes first, since it makes the subsequent patch much
simpler.

I took a quick look at it and will send the patches in reply to this
message.  It will probably be simplest if you pick them up and send a
series containing those two patches and the patch for mailmap support.

[PATCH 1/2] filter: introduce cgit_open_email_filter() wrapper

[john]

We provide email addresses to the email filter surrounded by angle
brackets, but we will soon remove these in our internal representation.
Introduce a wrapper so that we only have to add them in one place.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
It might be nice for this to be symmetric with a new
cgit_close_email_filter() thin wrapper as well, but I haven't bothered
with that in this first pass.
---
 cgit.h      | 2 ++
 filter.c    | 5 +++++
 ui-commit.c | 4 ++--
 ui-log.c    | 2 +-
 ui-refs.c   | 4 ++--
 5 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/cgit.h b/cgit.h
index 325432b..7331965 100644
--- a/cgit.h
+++ b/cgit.h
@@ -383,6 +383,8 @@ extern struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filterty
 extern void cgit_cleanup_filters(void);
 extern void cgit_init_filters(void);
 
+extern void cgit_open_email_filter(const char *email, const char *origin);
+
 extern void cgit_prepare_repo_env(struct cgit_repo * repo);
 
 extern int readfile(const char *path, char **buf, size_t *size);
diff --git a/filter.c b/filter.c
index 949c931..88098ba 100644
--- a/filter.c
+++ b/filter.c
@@ -454,3 +454,8 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 
 	die("Invalid filter type: %.*s", (int) len, cmd);
 }
+
+void cgit_open_email_filter(const char *email, const char *origin)
+{
+	cgit_open_filter(ctx.repo->email_filter, email, origin);
+}
diff --git a/ui-commit.c b/ui-commit.c
index 099d294..95958e0 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -47,7 +47,7 @@ void cgit_print_commit(char *hex, const char *prefix)
 	cgit_print_diff_ctrls();
 	html("<table summary='commit info' class='commit-info'>\n");
 	html("<tr><th>author</th><td>");
-	cgit_open_filter(ctx.repo->email_filter, info->author_email, "commit");
+	cgit_open_email_filter(info->author_email, "commit");
 	html_txt(info->author);
 	if (!ctx.cfg.noplainemail) {
 		html(" ");
@@ -59,7 +59,7 @@ void cgit_print_commit(char *hex, const char *prefix)
 				cgit_date_mode(DATE_ISO8601)));
 	html("</td></tr>\n");
 	html("<tr><th>committer</th><td>");
-	cgit_open_filter(ctx.repo->email_filter, info->committer_email, "commit");
+	cgit_open_email_filter(info->committer_email, "commit");
 	html_txt(info->committer);
 	if (!ctx.cfg.noplainemail) {
 		html(" ");
diff --git a/ui-log.c b/ui-log.c
index c97b8e0..7baf5e8 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -238,7 +238,7 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 			 oid_to_hex(&commit->object.oid), ctx.qry.vpath);
 	show_commit_decorations(commit);
 	html("</td><td>");
-	cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
+	cgit_open_email_filter(info->author_email, "log");
 	html_txt(info->author);
 	cgit_close_filter(ctx.repo->email_filter);
 
diff --git a/ui-refs.c b/ui-refs.c
index 75f2789..ffb9bb1 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -69,7 +69,7 @@ static int print_branch(struct refinfo *ref)
 	if (ref->object->type == OBJ_COMMIT) {
 		cgit_commit_link(info->subject, NULL, NULL, name, NULL, NULL);
 		html("</td><td>");
-		cgit_open_filter(ctx.repo->email_filter, info->author_email, "refs");
+		cgit_open_email_filter(info->author_email, "refs");
 		html_txt(info->author);
 		cgit_close_filter(ctx.repo->email_filter);
 		html("</td><td colspan='2'>");
@@ -148,7 +148,7 @@ static int print_tag(struct refinfo *ref)
 			cgit_close_filter(ctx.repo->email_filter);
 		}
 	} else if (ref->object->type == OBJ_COMMIT) {
-		cgit_open_filter(ctx.repo->email_filter, ref->commit->author_email, "refs");
+		cgit_open_email_filter(ref->commit->author_email, "refs");
 		html_txt(ref->commit->author);
 		cgit_close_filter(ctx.repo->email_filter);
 	}
-- 
2.10.0.rc0.142.g1e9f63b

[PATCH 2/2] Remove angle brackets from {author,committer}_email

[john]

This matches the internal representation in libgit.a, so it will make it
much easier to use Git's mailmap code.

The change in ui-atom.c isn't strictly necessary since the code copes
with email addresses both with and without angle brackets, but it's a
nice simplification since we know that the email address will always be
provided in the correct form.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 filter.c    | 10 +++++++++-
 parsing.c   |  6 +-----
 ui-atom.c   | 13 +------------
 ui-commit.c |  6 ++++--
 4 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/filter.c b/filter.c
index 88098ba..ba9000a 100644
--- a/filter.c
+++ b/filter.c
@@ -457,5 +457,13 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 
 void cgit_open_email_filter(const char *email, const char *origin)
 {
-	cgit_open_filter(ctx.repo->email_filter, email, origin);
+	struct strbuf sb = STRBUF_INIT;
+
+	/* Don't bother allocating any memory if we don't have a filter. */
+	if (!ctx.repo->email_filter)
+		return;
+
+	strbuf_addf(&sb, "<%s>", email);
+	cgit_open_filter(ctx.repo->email_filter, sb.buf, origin);
+	strbuf_release(&sb);
 }
diff --git a/parsing.c b/parsing.c
index 9dacb16..352338d 100644
--- a/parsing.c
+++ b/parsing.c
@@ -72,14 +72,10 @@ static char *substr(const char *head, const char *tail)
 static void parse_user(const char *t, char **name, char **email, unsigned long *date, int *tz)
 {
 	struct ident_split ident;
-	unsigned email_len;
 
 	if (!split_ident_line(&ident, t, strchrnul(t, '\n') - t)) {
 		*name = substr(ident.name_begin, ident.name_end);
-
-		email_len = ident.mail_end - ident.mail_begin;
-		*email = xmalloc(strlen("<") + email_len + strlen(">") + 1);
-		sprintf(*email, "<%.*s>", email_len, ident.mail_begin);
+		*email = substr(ident.mail_begin, ident.mail_end);
 
 		if (ident.date_begin)
 			*date = strtoul(ident.date_begin, NULL, 10);
diff --git a/ui-atom.c b/ui-atom.c
index 41838d3..7c17d6a 100644
--- a/ui-atom.c
+++ b/ui-atom.c
@@ -15,7 +15,6 @@ static void add_entry(struct commit *commit, const char *host)
 {
 	char delim = '&';
 	char *hex;
-	char *mail, *t, *t2;
 	struct commitinfo *info;
 
 	info = cgit_parse_commit(commit);
@@ -35,19 +34,9 @@ static void add_entry(struct commit *commit, const char *host)
 		html("</name>\n");
 	}
 	if (info->author_email && !ctx.cfg.noplainemail) {
-		mail = xstrdup(info->author_email);
-		t = strchr(mail, '<');
-		if (t)
-			t++;
-		else
-			t = mail;
-		t2 = strchr(t, '>');
-		if (t2)
-			*t2 = '\0';
 		html("<email>");
-		html_txt(t);
+		html_txt(info->author_email);
 		html("</email>\n");
-		free(mail);
 	}
 	html("</author>\n");
 	html("<published>");
diff --git a/ui-commit.c b/ui-commit.c
index 95958e0..061a596 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -50,8 +50,9 @@ void cgit_print_commit(char *hex, const char *prefix)
 	cgit_open_email_filter(info->author_email, "commit");
 	html_txt(info->author);
 	if (!ctx.cfg.noplainemail) {
-		html(" ");
+		html(" &lt;");
 		html_txt(info->author_email);
+		html("&gt;");
 	}
 	cgit_close_filter(ctx.repo->email_filter);
 	html("</td><td class='right'>");
@@ -62,8 +63,9 @@ void cgit_print_commit(char *hex, const char *prefix)
 	cgit_open_email_filter(info->committer_email, "commit");
 	html_txt(info->committer);
 	if (!ctx.cfg.noplainemail) {
-		html(" ");
+		html(" &lt;");
 		html_txt(info->committer_email);
+		html("&gt;");
 	}
 	cgit_close_filter(ctx.repo->email_filter);
 	html("</td><td class='right'>");
-- 
2.10.0.rc0.142.g1e9f63b

[PATCH] Add support for git's mailmap.

[smithj4]

On 08/25/2016 04:12 PM, John Keeping wrote:
> On Thu, Aug 25, 2016 at 03:22:35PM -0400, Jason A. Smith wrote:
>> On 08/24/2016 06:30 PM, John Keeping wrote:
>>> On Wed, Aug 24, 2016 at 05:27:31PM -0400, Jason A. Smith wrote:
>>>> Are you proposing to remove the angle brackets around the email address
>>>> everywhere in cgit strings, to match git, which I think is done in the
>>>> parse_user function? Then use git's map_user directly and create a
>>>> cgit_print_ident function that inserts the angle brackets when printed?
>>>> Then also making local copies of the pointers so freeing the info
>>>> structure doesn't break the mailmap? I think this would be a much larger
>>>> change beyond just supporting the mailmap.
>>>
>>> Yes, pretty much exactly this.  There's only a small number of sites
>>> that actually use author_email and committer_email, most of which are
>>> already printing something, so the fallout shouldn't be too bad.
>>>
>>> The only other cases are opening the email filter, which is why I
>>> suggested wrapping at least part of that process in a new function.
>>
>> So how do you propose splitting the patches to accomplish this? One
>> patch to add mailmap support and another patch to modify how angle
>> brackets are handled in the email addresses? Which patch goes first and
>> who will fix the angle brackets? Are you volunteering? ;)
>
> The cleanup goes first, since it makes the subsequent patch much
> simpler.
>
> I took a quick look at it and will send the patches in reply to this
> message.  It will probably be simplest if you pick them up and send a
> series containing those two patches and the patch for mailmap support.
>

Hi John,

Thanks for your quick help and patients with this, I am new to git 
workflows via email. I re-factored my mailmap patch on top of your two 
patches and will send them in a new thread. I still kept a small 
map_user() wrapper since the callers don't need the string lengths. Let 
me know if this looks good or if there are anymore comments/changes needed.

~Jason