Re: [PATCH v3 3/4] Add map_user() and clear_mailmap() to mailmap
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 4, 2009, 06:50 UTC
- Message-ID
- <7v3aeu66hv.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <53facc1773745ae477f15103718e08ea6a5ae58d.1233584536.git.marius@trolltech.com>
Marius Storm-Olsen <marius@trolltech.com> writes:
Show 13 quoted lines
> map_user() allows to lookup and replace both email and > name of a user, based on a new style mailmap file. > The possible mailmap definitions now are: > proper_name <commit_email> // Old style > proper_name <proper_email> <commit_email> // New style > proper_name <proper_email> commit_name <commit_email> // New style > > map_email() operates the same as before, with the > exception that it also will to try to match on a name > passed in through the name return buffer. > > clear_mailmap() is needed to now clear the more complex > mailmap structure.
Use of ample paragraph breaks would make it much easier to eyes.
map_user() allows to lookup and replace both email and name of a user,
based on a new style mailmap file.The possible mailmap definitions now are:
proper_name <commit_email> # Old style
proper_name <proper_email> <commit_email> # New style
proper_name <proper_email> commit_name <commit_email> # New style map_email() operates the same as before, with the exception that it also
will to try to match on a name passed in through the name return buffer.clear_mailmap() is needed to now clear the more complex mailmap structure.
Also, the example shows how lines would look line in a mailmap file. I would avoid giving a false impression that the parser should take C++ style comment introducer // by using '#' which is a documented one (I suspect anything that follows the last angle bracket is simply ignored, though).
> @@ -86,6 +95,27 @@ Jane Doe <jane@desktop.(none)> > Joe R. Developer <joe@random.com>
This context line was updated a few days ago (not a big deal, just in case you didn't know).
Show 14 quoted lines
> diff --git a/mailmap.c b/mailmap.c
> index 5aaee91..f593ff0 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -2,7 +2,87 @@
> #include "string-list.h"
> #include "mailmap.h"
>
> +#define DEBUG_MAILMAP 0
> +#if DEBUG_MAILMAP
> +#define debug_mm(...) fprintf(stderr, __VA_ARGS__)
> +#else
> +inline void debug_mm(const char *format, ...) {}
> +#endif"static inline void ...";
Show 7 quoted lines
> @@ -37,25 +117,65 @@ static int read_single_mailmap(struct string_list *map, const char *filename, ch >... > + /* Locate 2nd name and email. Possible mappings in mailmap file are: > + * proper_name <commit_email> > + * proper_name <proper_email> <commit_email> > + * proper_name <proper_email> commit_name <commit_email> > + */
/*
* We tend to write a multi line comment block
* like this.
*/Show 32 quoted lines
> + do {
> + email2 = name2 = 0;
> + right_bracket1 += 1;
> + if ((left_bracket2 = strchr(right_bracket1, '<')) == NULL)
> + continue;
> + if ((right_bracket2 = strchr(left_bracket2 + 1, '>')) == NULL)
> + continue;
> + if (right_bracket2 == left_bracket2 + 1)
> + continue;
> + for (end_of_name = left_bracket2;
> + end_of_name != right_bracket1 && isspace(end_of_name[-1]);
> + end_of_name--)
> + ; /* keep on looking for name end */
> + for (;
> + end_of_name != right_bracket1 && isspace(right_bracket1[0]);
> + right_bracket1++)
> + ; /* keep on looking for name start */
> + if (end_of_name != right_bracket1) {
> + name2 = xmalloc(end_of_name - right_bracket1 + 1);
> + strlcpy(name2, right_bracket1, end_of_name - right_bracket1 + 1);
> + char *tmp = name1;
> + name1 = name2;
> + name2 = tmp;
> + }
> + email2 = xmalloc(right_bracket2 - left_bracket2);
> + for (i = 0; i < right_bracket2 - left_bracket2 - 1; i++)
> + email2[i] = tolower(left_bracket2[i + 1]);
> + email2[right_bracket2 - left_bracket2 - 1] = '\0';
> + char *tmp = email1;
> + email1 = email2;
> + email2 = tmp;
> + } while(0);Yuck. Is it just me or is this new codeblock especially denser than existing code? I wonder use of a few smaller helper functions (that the compiler may be able to inline without being told for us) would make this easier to read without funny-looking "do { ... if (...) continue; } while (0)" trick?
Two "char *tmp" in this scope are both decl-after-statement errors.