From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Wed, 24 Aug 2016 23:30:30 +0100 Subject: [PATCH] Add support for git's mailmap. In-Reply-To: References: <20160824192329.wbiroty26davnidh@john.keeping.me.uk> Message-ID: <20160824223030.msh3y7woqiml5api@john.keeping.me.uk> 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.