Re: [PATCH v4 1/4] Add log.mailmap as configurational option for mailmap location

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano said the following on 05.02.2009 18:44:
Marius Storm-Olsen <marius@xxxxxxxxxxxxx> writes:

diff --git a/config.c b/config.c
index 790405a..9ebcbbe 100644
--- a/config.c
+++ b/config.c
@@ -565,6 +565,13 @@ static int git_default_branch_config(const char *var, const char *value)
 	return 0;
 }
+static int git_default_log_config(const char *var, const char *value)
+{
+	if (!strcmp(var, "log.mailmap"))
+		return git_config_string(&git_log_mailmap, var, value);
+	return 0;
+}
+
 int git_default_config(const char *var, const char *value, void *dummy)
 {
 	if (!prefixcmp(var, "core."))
@@ -579,6 +586,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
 	if (!prefixcmp(var, "branch."))
 		return git_default_branch_config(var, value);
+ if (!prefixcmp(var, "log."))
+		return git_default_log_config(var, value);
+

The placement of this looked *really* wrong, as mailmap is not
*that* important to most of the commands.

Initially I wondered if this should be better done inside existing git_log_config(). I suspect that the reason you didn't do so is
because you would want to use this also in blame, which is not part
of the log family, and does not use git_log_config() (nor it
should).

Which probably means that the code can stay here (it is just two
strcmp and assignment to a pointer variable), but also suggests
that log.mailmap is perhaps misnamed.


Correct, that was my reasoning behind it. Since shortlog is the only place in the documentation where mailmap is *directly* mentioned, it feels slightly tied to log. But, since blame and pretty.c also reference it, I needed the configuration option to be read as default.

Given that in total shortlog, blame, log, diff-tree, rev-list, show and whatchanged use it (the latter 5 through the pretty option), I'm tempted to say that it justifies its own option (mailmap.file?); but it would still have to be handled by git_default_config(). Renaming it would give it stronger reason to *be there* though.

I'm fine either way, really. Though, I think if we rename the option, it also justifies pulling the mailmap documentation out of git-shortlog.txt, into its own file, and link to it from shortlog, and the other commands which use it (git-blame.txt and pretty-format.txt)

I'll happily do the job, if "yay", or leave it as is if "nay".
Either way, feel free to rename log.mailmap to something else.

--
.marius
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux