Eric Sunshine <[email protected]> writes:
> +DESCRIPTION
> +-----------
> +
> +For each ``Name $$<email@address>$$'' or ``$$<email@address>$$'' provided on
> +the command-line or standard input (when using `--stdin`), prints a line with
> +the canonical contact information for that person according to the 'mailmap'
> +(see "Mapping Authors" below). If no mapping exists for the person, echoes
> the
> +provided contact information.
OK. The last part needed a reading and a half for me to realize the
"echoes the provided contact information" is the same thing as "the
"input string is printed as-is", especially because "contact
information" is not defined anywhere in the previous part of the
DESCRIPTION section, though. I might be worth starting the
paragraph with:
For each contact information (either in the form of ``Name
<user@host>'' or ...)
in order to clarify that the two forms of input is what you call
"contact information".
> diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c
> new file mode 100644
> index 0000000..c36a61c
> --- /dev/null
> +++ b/builtin/check-mailmap.c
> @@ -0,0 +1,69 @@
> +#include "builtin.h"
> +#include "mailmap.h"
> +#include "parse-options.h"
> +#include "string-list.h"
> +
> +static int use_stdin;
> +static int null_out;
Is there a reason why the variable name should not match that of the
corresponding variables in check-ignore and check-attr, which you
copied the bulk of the code from?
If there isn't, use "null_term_line" like they do.
> +static const char * const check_mailmap_usage[] = {
> +N_("git check-mailmap [options] <contact>..."),
> +NULL
> +};
This mis-indentation looks somewhat unfortunate, but they are shared
with other check-blah, and they are meant to contain a rather long
string, so let's keep it like so.
> +static void check_mailmap(struct string_list *mailmap, const char *contact)
> +{
> + const char *name, *mail;
> + size_t namelen, maillen;
> + struct ident_split ident;
> + char term = null_out ? '\0' : '\n';
Is there a reason why the variable name "term" should not match that
of the corresponding variables in check-ignore and check-attr, which
you copied the bulk of the code from?
If there isn't, use "line_termination" like they do.
> + if (split_ident_line(&ident, contact, strlen(contact)))
> + die(_("unable to parse contact: %s"), contact);
> +
> + name = ident.name_begin;
> + namelen = ident.name_end - ident.name_begin;
> + mail = ident.mail_begin;
> + maillen = ident.mail_end - ident.mail_begin;
> +
> + map_user(mailmap, &mail, &maillen, &name, &namelen);
> +
> + if (namelen)
> + printf("%.*s ", (int)namelen, name);
> + printf("<%.*s>%c", (int)maillen, mail, term);
> +}
> +
> +int cmd_check_mailmap(int argc, const char **argv, const char *prefix)
> +{
> + int i;
> + struct string_list mailmap = STRING_LIST_INIT_NODUP;
> +
> + git_config(git_default_config, NULL);
> + argc = parse_options(argc, argv, prefix, check_mailmap_options,
> + check_mailmap_usage, 0);
It is a bit distracting that the second line that is not indented
enough. Either (1) with enough HT and with minimum number of SP,
align "argc" and "check_mailmap_usage", or (2) with minimum number
of HT and no SP, push "check_mailmap_usage" to the right of opening
parenthesis of "(argc". Our code tend to prefer (1).
> + if (argc == 0 && !use_stdin)
> + die(_("no contacts specified"));
> +
> + read_mailmap(&mailmap, NULL);
> +
> + for (i = 0; i < argc; ++i)
> + check_mailmap(&mailmap, argv[i]);
> + maybe_flush_or_die(stdout, "stdout");
> +
> + if (use_stdin) {
> + struct strbuf buf = STRBUF_INIT;
> + while (strbuf_getline(&buf, stdin, '\n') != EOF) {
> + check_mailmap(&mailmap, buf.buf);
> + maybe_flush_or_die(stdout, "stdout");
> + }
> + strbuf_release(&buf);
> + }
> +
> + clear_mailmap(&mailmap);
> + return 0;
> +}
> diff --git a/command-list.txt b/command-list.txt
> index bf83303..08b04e2 100644
> --- a/command-list.txt
> +++ b/command-list.txt
> @@ -13,6 +13,7 @@ git-bundle mainporcelain
> git-cat-file plumbinginterrogators
> git-check-attr purehelpers
> git-check-ignore purehelpers
> +git-check-mailmap purehelpers
> git-checkout mainporcelain common
> git-checkout-index plumbingmanipulators
> git-check-ref-format purehelpers
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index ebc40d4..c118550 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -648,6 +648,7 @@ __git_list_porcelain_commands ()
> cat-file) : plumbing;;
> check-attr) : plumbing;;
> check-ignore) : plumbing;;
> + check-mailmap) : plumbing;;
> check-ref-format) : plumbing;;
> checkout-index) : plumbing;;
> commit-tree) : plumbing;;
> diff --git a/git.c b/git.c
> index 4359086..02c3035 100644
> --- a/git.c
> +++ b/git.c
> @@ -324,6 +324,7 @@ static void handle_internal_command(int argc, const char
> **argv)
> { "cat-file", cmd_cat_file, RUN_SETUP },
> { "check-attr", cmd_check_attr, RUN_SETUP },
> { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE
> },
> + { "check-mailmap", cmd_check_mailmap, RUN_SETUP },
As we can read from HEAD:.mailmap these days, the patch is correct
that it does not require NEED_WORK_TREE here.
Thanks.
> { "check-ref-format", cmd_check_ref_format },
> { "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
> { "checkout-index", cmd_checkout_index,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
