Re: [RFC/PATCH] log: add log.firstparent option

[Jeff King <peff@peff.net>]

On Fri, Jul 24, 2015 at 08:07:49AM -0700, Junio C Hamano wrote:

> > I am not entirely convinced this won't bite somebody who gets a sha1
> > from some other source, and then wants to run:
> >
> >   git log $some_other_sha1
> >
> > who might be quite confused to start a first-parent traversal from
> > somewhere other than the tip of "master" or the tip of a topic branch.
> 
> Yeah, you actually convinced me reasonably well that it would
> happen.  I'd never use it myself.  If people want to shoot
> themselves in the foot, be my guest ;-)
> 
> Perhaps we should drop this, and give a shorter synonym to the
> option?

I'm still on the fence to have the config kick in only for HEAD.
Something like (on top of my other patch, and the tests would still need
adjusted):

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e9c3763..f2b6a21 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1802,9 +1802,11 @@ log.mailmap::
 	If true, makes linkgit:git-log[1], linkgit:git-show[1], and
 	linkgit:git-whatchanged[1] assume `--use-mailmap`.
 
-log.firstparent::
-	If true, linkgit:git-log[1] will default to `--first-parent`;
-	can be overridden by supplying `--no-first-parent`.
+log.defaultImpliesFirstParent::
+	If true, linkgit:git-log[1] will default to `--first-parent`
+	when showing the default ref (i.e., if you run only `git
+	log` to show `HEAD`, but not `git log $sha1`). Can be overridden
+	by supplying `--no-first-parent`.
 
 mailinfo.scissors::
 	If true, makes linkgit:git-mailinfo[1] (and therefore
diff --git a/builtin/log.c b/builtin/log.c
index 3e9b034..2bdb3fc 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,7 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
-static int default_first_parent;
+static int default_implies_first_parent;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -110,7 +110,6 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	rev->abbrev_commit = default_abbrev_commit;
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
-	rev->first_parent_only = default_first_parent;
 	DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
 
 	if (default_date_mode)
@@ -398,8 +397,8 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		use_mailmap_config = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "log.firstparent")) {
-		default_first_parent = git_config_bool(var, value);
+	if (!strcmp(var, "log.defaultimpliesfirstparent")) {
+		default_implies_first_parent = git_config_bool(var, value);
 		return 0;
 	}
 
@@ -504,6 +503,8 @@ static int show_tree_object(const unsigned char *sha1,
 
 static void show_rev_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)
 {
+	if (default_implies_first_parent && !rev->pending.nr)
+		rev->first_parent_only = 1;
 	if (rev->ignore_merges) {
 		/* There was no "-m" on the command line */
 		rev->ignore_merges = 0;


It feels somewhat magical, but at least the config option name makes it
painfully clear exactly when it would kick in. I dunno. I am happy
enough for myself to just run "--first-parent" when that is what I want
to see. Giving it a shorter name would not help much, I think. It is not
the number of characters, but the fact that most people do not _know_
that --first-parent exists in the first place, or that it would be
useful in this case. I hoped with a config option it might become
something projects could recommend to their users[1] if the project has a
matching workflow. But maybe we could also rely on those same projects
to educate their users.

-Peff

[1] And if not an official recommendation from a project, this is the
    sort of "tips and tricks" information that may spread informally. But in
    theory so could knowledge of --first-parent.