threads / patch / 39761

patchlog: add log.follow config option

Subject: [PATCH] log: add log.follow config option

## tl;dr

6 messages between Jul 1, 2015 and Jul 3, 2015. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

David Turner· Jul 1, 2015, 19:03 UTC · lore

Many users prefer to always use --follow with logs. Rather than aliasing the command, an option might be more convenient for some.

Signed-off-by: David Turner <dturner@twopensource.com>
---
Why not just alias log=log --follow?

At Twitter, we manage git config centrally, but we also allow users to add their own aliases. We would like to turn log.follow on globally, while not messing up any aliases users already have for log.

Also, some users might have different log aliases for different repositories, but want to manage --follow globally.

And in the future, we might want to make log --follow the default (it is what I usually want), so it would be nice to provide a way for users to turn that off globally from a config option.

 builtin/log.c                       |  7 +++++++
 t/t4206-log-follow-harder-copies.sh | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+)
Show changes to 2 files +30 −0

builtin/log.c, t/t4206-log-follow-harder-copies.sh

diff --git a/builtin/log.c b/builtin/log.c
index 8781049..11b8d82 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_follow = 0;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 {
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
+	if (default_follow)
+		DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);
 	rev->verbose_header = 1;
 	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->diffopt.stat_width = -1; /* use full terminal width */
@@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
 		default_show_root = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "log.follow")) {
+		default_follow = git_config_bool(var, value);
+		return 0;
+	}
 	if (skip_prefix(var, "color.decorate.", &slot_name))
 		return parse_decorate_color_config(var, slot_name, value);
 	if (!strcmp(var, "log.mailmap")) {
diff --git a/t/t4206-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh
index ad29e65..6b2f3b9 100755
--- a/t/t4206-log-follow-harder-copies.sh
+++ b/t/t4206-log-follow-harder-copies.sh
@@ -53,4 +53,27 @@ test_expect_success \
     'validate the output.' \
     'compare_diff_patch current expected'
 
+test_expect_success \
+    'git config log.follow works like --follow' \
+    'test_config log.follow true &&
+     git log --name-status --pretty="format:%s"  path1 > current'
+
+test_expect_success \
+    'validate the output.' \
+    'compare_diff_patch current expected'
+
+test_expect_success \
+    'git config log.follow is overridden by --no-follow' \
+    'test_config log.follow true &&
+     git log --no-follow --name-status --pretty="format:%s"  path1 > current'
+
+cat >expected <<\EOF
+Copy path1 from path0
+A	path1
+EOF
+
+test_expect_success \
+    'validate the output.' \
+    'compare_diff_patch current expected'
+
 test_done
-- 
2.0.5.499.g01f6352.dirty-twtrsrc
Matthieu Moy· Jul 1, 2015, 21:16 UTC · re: David Turner · lore

Re: [PATCH] log: add log.follow config option

David Turner <dturner@twopensource.com> writes:
Show 18 quoted lines
> diff --git a/builtin/log.c b/builtin/log.c
> index 8781049..11b8d82 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
>  
>  static int default_abbrev_commit;
>  static int default_show_root = 1;
> +static int default_follow = 0;
>  static int decoration_style;
>  static int decoration_given;
>  static int use_mailmap_config;
> @@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
>  {
>  	if (fmt_pretty)
>  		get_commit_format(fmt_pretty, rev);
> +	if (default_follow)
> +		DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);

Doesn't it activate --follow all the time, including when the user provides several paths?

In this case, you get this:

$ git log --follow *.c fatal: --follow requires exactly one pathspec

So activating --follow for all "git log" calls would prevent log from being used with several pathspecs.

Or, do you have a preparation patch that allows --follow with multiple pathspecs? ;-)

In any case, you have to test "git log -- path1 path2" with the option activated.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· Jul 1, 2015, 21:19 UTC · re: Matthieu Moy · lore

Re: [PATCH] log: add log.follow config option

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 8 quoted lines
> So activating --follow for all "git log" calls would prevent log from
> being used with several pathspecs.
>
> Or, do you have a preparation patch that allows --follow with multiple
> pathspecs? ;-)
>
> In any case, you have to test "git log -- path1 path2" with the option
> activated.
Or more commonly, just "git log" with no pathspec.

I also think that it is a bad idea to force "log --follow" to people before it is made into a true feature; as it stands, it is merely a "checkbox" item. It has too severe limitation to be used seriously in real projects, unless your history is completely linear.

cf.
  http://thread.gmane.org/gmane.comp.version-control.git/269357/focus=269433
David Turner· Jul 2, 2015, 18:43 UTC · re: Junio C Hamano · lore

Re: [PATCH] log: add log.follow config option

On Wed, 2015-07-01 at 14:19 -0700, Junio C Hamano wrote:
Show 21 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
> > So activating --follow for all "git log" calls would prevent log from
> > being used with several pathspecs.
> >
> > Or, do you have a preparation patch that allows --follow with multiple
> > pathspecs? ;-)
> >
> > In any case, you have to test "git log -- path1 path2" with the option
> > activated.
> 
> Or more commonly, just "git log" with no pathspec.
> 
> I also think that it is a bad idea to force "log --follow" to people
> before it is made into a true feature; as it stands, it is merely a
> "checkbox" item.  It has too severe limitation to be used seriously
> in real projects, unless your history is completely linear.
> 
> cf.
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/269357/focus=269433
Thanks.
Twitter's history is almost completely linear, so it's useful for us.  

Since it looks like the patch won't be useful outside of our context, I'll just rewrite it to check the pathspec count, and not upstream it until follow becomes more general.

Matthieu Moy· Jul 3, 2015, 07:17 UTC · re: David Turner · lore

Re: [PATCH] log: add log.follow config option

David Turner <dturner@twopensource.com> writes:
Show 5 quoted lines
> Twitter's history is almost completely linear, so it's useful for us.  
>
> Since it looks like the patch won't be useful outside of our context,
> I'll just rewrite it to check the pathspec count, and not upstream it
> until follow becomes more general.

As long as it's an opt-in and that the documentation states the limitations clearly enough, I think it makes sense to me to have this upstream.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· Jul 3, 2015, 17:47 UTC · re: Matthieu Moy · lore

Re: [PATCH] log: add log.follow config option

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 11 quoted lines
> David Turner <dturner@twopensource.com> writes:
>
>> Twitter's history is almost completely linear, so it's useful for us.  
>>
>> Since it looks like the patch won't be useful outside of our context,
>> I'll just rewrite it to check the pathspec count, and not upstream it
>> until follow becomes more general.
>
> As long as it's an opt-in and that the documentation states the
> limitations clearly enough, I think it makes sense to me to have this
> upstream.

Perhaps. But at least "log -- 1 2" and "log --" should not be broken for those that set the configuration.

Unless you are counting these as also "limitations", that is.

← back to recent threads