{"thread":{"id":"39802","subject":"[PATCH v2] log: add log.follow config option","startedAt":"2015-07-07T18:40:40Z","lastAt":"2015-07-08T16:49:06Z","messageCount":7,"participants":["David Turner","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"265759","messageId":"1436294440-20273-1-git-send-email-dturner@twopensource.com","threadId":"39802","inReplyTo":null,"subject":"[PATCH v2] log: add log.follow config option","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-07T18:40:40Z","receivedAt":"2015-07-07T18:40:40Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: David Turner <dturner@twitter.com>\n\nMany users prefer to always use --follow with logs.  Rather than\naliasing the command, an option might be more convenient for some.\n---\n Documentation/git-log.txt           |  7 +++++++\n builtin/log.c                       |  7 +++++++\n diff.c                              |  5 +++--\n diff.h                              |  1 +\n revision.c                          | 15 ++++++++++++---\n t/t4206-log-follow-harder-copies.sh | 35 +++++++++++++++++++++++++++++++++++\n 6 files changed, 65 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 5692945..5a23085 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -184,6 +184,13 @@ log.date::\n \t`--date` option.)  Defaults to \"default\", which means to write\n \tdates like `Sat May 8 19:35:34 2010 -0500`.\n \n+log.follow::\n+\tIf a single file argument is given to git log, it will act as\n+\tif the `--follow` option was also used.  This adds no new\n+\tfunctionality over what --follow already provides (in other words,\n+\tit cannot be used to follow multiple files).  It's just a\n+\tconvenience.\n+\n log.showRoot::\n \tIf `false`, `git log` and related commands will not treat the\n \tinitial commit as a big creation event.  Any root commits in\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8781049..9b6abef 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;\n \n static int default_abbrev_commit;\n static int default_show_root = 1;\n+static int default_follow = 0;\n static int decoration_style;\n static int decoration_given;\n static int use_mailmap_config;\n@@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)\n {\n \tif (fmt_pretty)\n \t\tget_commit_format(fmt_pretty, rev);\n+\tif (default_follow)\n+\t\tDIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);\n \trev->verbose_header = 1;\n \tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n \trev->diffopt.stat_width = -1; /* use full terminal width */\n@@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tdefault_show_root = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"log.follow\")) {\n+\t\tdefault_follow = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (skip_prefix(var, \"color.decorate.\", &slot_name))\n \t\treturn parse_decorate_color_config(var, slot_name, value);\n \tif (!strcmp(var, \"log.mailmap\")) {\ndiff --git a/diff.c b/diff.c\nindex 87b16d5..135b222 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3815,9 +3815,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_SET(options, FIND_COPIES_HARDER);\n \telse if (!strcmp(arg, \"--follow\"))\n \t\tDIFF_OPT_SET(options, FOLLOW_RENAMES);\n-\telse if (!strcmp(arg, \"--no-follow\"))\n+\telse if (!strcmp(arg, \"--no-follow\")) {\n \t\tDIFF_OPT_CLR(options, FOLLOW_RENAMES);\n-\telse if (!strcmp(arg, \"--color\"))\n+\t\tDIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);\n+\t} else if (!strcmp(arg, \"--color\"))\n \t\toptions->use_color = 1;\n \telse if (skip_prefix(arg, \"--color=\", &arg)) {\n \t\tint value = git_config_colorbool(NULL, arg);\ndiff --git a/diff.h b/diff.h\nindex c7ad42a..f7208ad 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -91,6 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n #define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_TOUCHED(opts, flag)    ((opts)->touched_flags & DIFF_OPT_##flag)\ndiff --git a/revision.c b/revision.c\nindex 3ff8723..ae6d4c3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \n \tif (revs->prune_data.nr) {\n \t\tcopy_pathspec(&revs->pruning.pathspec, &revs->prune_data);\n-\t\t/* Can't prune commits with rename following: the paths change.. */\n-\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n-\t\t\trevs->prune = 1;\n+\n \t\tif (!revs->full_diff)\n \t\t\tcopy_pathspec(&revs->diffopt.pathspec,\n \t\t\t\t      &revs->prune_data);\n+\n+\t\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n+\t\t    revs->diffopt.pathspec.nr == 1)\n+\t\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n+\n+\t\t/* Can't prune commits with rename following: the paths change.. */\n+\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {\n+\t\t\trevs->prune = 1;\n+\t\t} else {\n+\t\t\trevs->diff = 1;\n+\t\t}\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\ndiff --git a/t/t4206-log-follow-harder-copies.sh b/t/t4206-log-follow-harder-copies.sh\nindex ad29e65..5355398 100755\n--- a/t/t4206-log-follow-harder-copies.sh\n+++ b/t/t4206-log-follow-harder-copies.sh\n@@ -53,4 +53,39 @@ test_expect_success \\\n     'validate the output.' \\\n     'compare_diff_patch current expected'\n \n+test_expect_success \\\n+    'git config log.follow works like --follow' \\\n+    'test_config log.follow true &&\n+     git log --name-status --pretty=\"format:%s\"  path1 > current'\n+\n+test_expect_success \\\n+    'validate the output.' \\\n+    'compare_diff_patch current expected'\n+\n+test_expect_success \\\n+    'git config log.follow does not die with multiple paths' \\\n+    'test_config log.follow true &&\n+     git log path0 path1 > current &&\n+     wc -l current'\n+\n+test_expect_success \\\n+    'git config log.follow does not die with no paths' \\\n+    'test_config log.follow true &&\n+     git log -- > current &&\n+     wc -l current'\n+\n+test_expect_success \\\n+    'git config log.follow is overridden by --no-follow' \\\n+    'test_config log.follow true &&\n+     git log --no-follow --name-status --pretty=\"format:%s\"  path1 > current'\n+\n+cat >expected <<\\EOF\n+Copy path1 from path0\n+A\tpath1\n+EOF\n+\n+test_expect_success \\\n+    'validate the output.' \\\n+    'compare_diff_patch current expected'\n+\n test_done\n-- \n2.0.5.499.g01f6352.dirty-twtrsrc\n"},{"id":"265772","messageId":"vpqlherlk4j.fsf@anie.imag.fr","threadId":"39802","inReplyTo":"1436294440-20273-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v2] log: add log.follow config option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-07-07T21:42:52Z","receivedAt":"2015-07-07T21:42:52Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Hi,\n\nThanks for your patch. Below are some comments. Some of them are just me\nthinking out loudly (don't take it badly if I'm being negative), some\nare more serious, but all are fixable.\n\nDavid Turner <dturner@twopensource.com> writes:\n\n> From: David Turner <dturner@twitter.com>\n\nIf you configure your commiter id and your From field to the same value,\nyou can avoid this distracting \"From:\" header.\n\nYou're lacking a Signed-off-by trailer.\n\n> +log.follow::\n> +\tIf a single file argument is given to git log, it will act as\n> +\tif the `--follow` option was also used.  This adds no new\n> +\tfunctionality over what --follow already provides (in other words,\n> +\tit cannot be used to follow multiple files).  It's just a\n> +\tconvenience.\n\nMissing `...` around the second --follow.\n\nI would write this as:\n\n\tThis has the same limitations as --follow, i.e. it cannot be\n\tused to follow multiple files and does not work well on\n\tnon-linear history.\n\nand drop the \"it's just a convenience\" part.\n\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;\n>  \n>  static int default_abbrev_commit;\n>  static int default_show_root = 1;\n> +static int default_follow = 0;\n\nI tend to disagree with this rule, but in Git we usually omit the \"= 0\"\nfor static variables, which are already initialized to 0 by default.\n\n> +\t\t/* Can't prune commits with rename following: the paths change.. */\n> +\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {\n> +\t\t\trevs->prune = 1;\n> +\t\t} else {\n> +\t\t\trevs->diff = 1;\n> +\t\t}\n\nUseless braces.\n\n> +     git log --name-status --pretty=\"format:%s\"  path1 > current'\n\nWhitespace: here an elsewhere, you have two spaces before path1, and we\nusually stick the > to the filename like >current.\n\n> --- a/t/t4206-log-follow-harder-copies.sh\n> +++ b/t/t4206-log-follow-harder-copies.sh\n> @@ -53,4 +53,39 @@ test_expect_success \\\n>      'validate the output.' \\\n>      'compare_diff_patch current expected'\n>  \n> +test_expect_success \\\n> +    'git config log.follow works like --follow' \\\n> +    'test_config log.follow true &&\n> +     git log --name-status --pretty=\"format:%s\"  path1 > current'\n> +\n> +test_expect_success \\\n> +    'validate the output.' \\\n> +    'compare_diff_patch current expected'\n\nI would squash these two tests. As-is, the first one doesn't really test\nanything (well, just that git log doesn't crash with log.follow).\n\nI think you meant test_cmp, not compare_diff_patch, as you don't need\nthe \"similarity index\" and \"index ...\" filtering that compare_diff_patch\ndoes before test_cmp.\n\nThat said, I see that you are mimicking surrounding code, which is a\ngood thing for consistancy. I think the best would be to write tests in\nt4202-log.sh, which already tests the --follow option, and uses a more\nmodern coding style which you can mimick.\nt4206-log-follow-harder-copies.sh is really about finding copies in\n--follow. Another option is to start your series with a patch like\n\"t4206: modernize style\".\n\nOr you can just accept that the current style in this file is subpar and\ncontinue with it.\n\n> +test_expect_success \\\n> +    'git config log.follow does not die with multiple paths' \\\n> +    'test_config log.follow true &&\n> +     git log path0 path1 > current &&\n\nYou are creating 'current' but not using it.\n\n> +     wc -l current'\n\nWhat is the intent of this \"wc -l current\"? You're not checking its\noutput ...\n\n> +test_expect_success \\\n> +    'git config log.follow does not die with no paths' \\\n> +    'test_config log.follow true &&\n> +     git log -- > current &&\n\nOne more creation of current which isn't used ...\n\n> +     wc -l current'\n> +\n> +test_expect_success \\\n> +    'git config log.follow is overridden by --no-follow' \\\n> +    'test_config log.follow true &&\n> +     git log --no-follow --name-status --pretty=\"format:%s\"  path1 > current'\n\n... because you're overwriting it here.\n\n> +cat >expected <<\\EOF\n> +Copy path1 from path0\n> +A\tpath1\n> +EOF\n\nPut everything in test_expect_..., including creation of expected file.\nFor more info, read t/README and its Do's, don'ts & things to keep in\nmind section.\n\n> +test_expect_success \\\n> +    'validate the output.' \\\n> +    'compare_diff_patch current expected'\n> +\n>  test_done\n\nCheers,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"265774","messageId":"1436307106.5521.7.camel@twopensource.com","threadId":"39802","inReplyTo":"vpqlherlk4j.fsf@anie.imag.fr","subject":"Re: [PATCH v2] log: add log.follow config option","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-07T22:11:46Z","receivedAt":"2015-07-07T22:11:46Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-07-07 at 23:42 +0200, Matthieu Moy wrote:\n> Hi,\n> \n> Thanks for your patch. Below are some comments. Some of them are just me\n> thinking out loudly (don't take it badly if I'm being negative), some\n> are more serious, but all are fixable.\n\nThanks for the feedback!\n\n> David Turner <dturner@twopensource.com> writes:\n> \n> > From: David Turner <dturner@twitter.com>\n> \n> If you configure your commiter id and your From field to the same value,\n> you can avoid this distracting \"From:\" header.\n> \n> You're lacking a Signed-off-by trailer.\n\nOops.  Cherry-picked over from internal repo.  Will fix.\n\n<snip> (suggestions applied)\n\n> > +     git log --name-status --pretty=\"format:%s\"  path1 > current'\n> \n> Whitespace: here an elsewhere, you have two spaces before path1, and we\n> usually stick the > to the filename like >current.\n>\n> > --- a/t/t4206-log-follow-harder-copies.sh\n> > +++ b/t/t4206-log-follow-harder-copies.sh\n> > @@ -53,4 +53,39 @@ test_expect_success \\\n> >      'validate the output.' \\\n> >      'compare_diff_patch current expected'\n> >  \n> > +test_expect_success \\\n> > +    'git config log.follow works like --follow' \\\n> > +    'test_config log.follow true &&\n> > +     git log --name-status --pretty=\"format:%s\"  path1 > current'\n> > +\n> > +test_expect_success \\\n> > +    'validate the output.' \\\n> > +    'compare_diff_patch current expected'\n> \n> I would squash these two tests. As-is, the first one doesn't really test\n> anything (well, just that git log doesn't crash with log.follow).\n> \n> I think you meant test_cmp, not compare_diff_patch, as you don't need\n> the \"similarity index\" and \"index ...\" filtering that compare_diff_patch\n> does before test_cmp.\n> \n> That said, I see that you are mimicking surrounding code, which is a\n> good thing for consistancy. I think the best would be to write tests in\n> t4202-log.sh, which already tests the --follow option, and uses a more\n> modern coding style which you can mimick.\n> t4206-log-follow-harder-copies.sh is really about finding copies in\n> --follow. Another option is to start your series with a patch like\n> \"t4206: modernize style\".\n\nI'm going to move over to t4202.  My next re-roll will include fixes for\neverything you raised.  \n"},{"id":"265776","messageId":"xmqqsi8z1urz.fsf@gitster.dls.corp.google.com","threadId":"39802","inReplyTo":"1436294440-20273-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v2] log: add log.follow config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-07T22:13:04Z","receivedAt":"2015-07-07T22:13:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> diff --git a/revision.c b/revision.c\n> index 3ff8723..ae6d4c3 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>  \n>  \tif (revs->prune_data.nr) {\n>  \t\tcopy_pathspec(&revs->pruning.pathspec, &revs->prune_data);\n> -\t\t/* Can't prune commits with rename following: the paths change.. */\n> -\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n> -\t\t\trevs->prune = 1;\n> +\n>  \t\tif (!revs->full_diff)\n>  \t\t\tcopy_pathspec(&revs->diffopt.pathspec,\n>  \t\t\t\t      &revs->prune_data);\n> +\n> +\t\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n> +\t\t    revs->diffopt.pathspec.nr == 1)\n> +\t\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n> +\n> +\t\t/* Can't prune commits with rename following: the paths change.. */\n> +\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {\n> +\t\t\trevs->prune = 1;\n> +\t\t} else {\n> +\t\t\trevs->diff = 1;\n> +\t\t}\n>  \t}\n>  \tif (revs->combine_merges)\n>  \t\trevs->ignore_merges = 0;\n\nIt is unfortunate that we have to waste one DIFF_OPT bit and touch\nrevision.c for something that is \"just a convenience\".  Because\nsetup_revisions() does not have a way to let you flip the settings\ndepending on the number of pathspecs specified, I do not think of a\nsolution that does not have to touch a low level foundation part of\nthe codebase, which I'd really want to avoid.\n\nBut I wonder why your patch needs to touch so much.\n\nYour changes to other files make sure that diffopt has the\nDEFAULT_FOLLOW_RENAMES when (1) the configuration is set and (2) the\ncommand line did not override it with --no-follow.  And these look\nvery sensible.\n\nIsn't the only thing left to do in this codepath, after the DEFAULT_\nis set up correctly, to set FOLLOW_RENAMES when (1) DEFAULT_ is set\nand (2) you have only one path?\n\nAnd once FOLLOW_RENAMES is set or unset according to the end-user\nvisible semantics, i.e. \"just for a convenience, setting log.follow\nadds --follow to the command line if and only if there is only one\npathspec\", I do not see why existing code needs to be modified in\nany other way.\n\nIOW, I'd like to know why we need more than something like this\nchange to this file, instead of the above?  We didn't muck with\nrevs->diff in the original when FOLLOW_RENAMES was set, but now it\ndoes, for example.\n\ndiff --git a/revision.c b/revision.c\nindex 3ff8723..f7bd229 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2270,6 +2270,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\tgot_rev_arg = 1;\n \t}\n \n+\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n+\t    revs->diffopt.pathspec.nr == 1)\n+\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n+\n \tif (prune_data.nr) {\n \t\t/*\n \t\t * If we need to introduce the magic \"a lone ':' means no\n"},{"id":"265797","messageId":"1436318939.5521.32.camel@twopensource.com","threadId":"39802","inReplyTo":"xmqqsi8z1urz.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] log: add log.follow config option","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-07-08T01:28:59Z","receivedAt":"2015-07-08T01:28:59Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Tue, 2015-07-07 at 15:13 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> > diff --git a/revision.c b/revision.c\n> > index 3ff8723..ae6d4c3 100644\n> > --- a/revision.c\n> > +++ b/revision.c\n> > @@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n> >  \n> >  \tif (revs->prune_data.nr) {\n> >  \t\tcopy_pathspec(&revs->pruning.pathspec, &revs->prune_data);\n> > -\t\t/* Can't prune commits with rename following: the paths change.. */\n> > -\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n> > -\t\t\trevs->prune = 1;\n> > +\n> >  \t\tif (!revs->full_diff)\n> >  \t\t\tcopy_pathspec(&revs->diffopt.pathspec,\n> >  \t\t\t\t      &revs->prune_data);\n> > +\n> > +\t\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n> > +\t\t    revs->diffopt.pathspec.nr == 1)\n> > +\t\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n> > +\n> > +\t\t/* Can't prune commits with rename following: the paths change.. */\n> > +\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {\n> > +\t\t\trevs->prune = 1;\n> > +\t\t} else {\n> > +\t\t\trevs->diff = 1;\n> > +\t\t}\n> >  \t}\n> >  \tif (revs->combine_merges)\n> >  \t\trevs->ignore_merges = 0;\n> \n> It is unfortunate that we have to waste one DIFF_OPT bit and touch\n> revision.c for something that is \"just a convenience\".  Because\n> setup_revisions() does not have a way to let you flip the settings\n> depending on the number of pathspecs specified, I do not think of a\n> solution that does not have to touch a low level foundation part of\n> the codebase, which I'd really want to avoid.\n> \n> But I wonder why your patch needs to touch so much.\n> \n> Your changes to other files make sure that diffopt has the\n> DEFAULT_FOLLOW_RENAMES when (1) the configuration is set and (2) the\n> command line did not override it with --no-follow.  And these look\n> very sensible.\n> \n> Isn't the only thing left to do in this codepath, after the DEFAULT_\n> is set up correctly, to set FOLLOW_RENAMES when (1) DEFAULT_ is set\n> and (2) you have only one path?\n>\n> And once FOLLOW_RENAMES is set or unset according to the end-user\n> visible semantics, i.e. \"just for a convenience, setting log.follow\n> adds --follow to the command line if and only if there is only one\n> pathspec\", I do not see why existing code needs to be modified in\n> any other way.\n> \n> IOW, I'd like to know why we need more than something like this\n> change to this file, instead of the above?  We didn't muck with\n> revs->diff in the original when FOLLOW_RENAMES was set, but now it\n> does, for example.\n\nWe did, but we did it earlier.  But I can just rearrange the code.\n\n> diff --git a/revision.c b/revision.c\n> index 3ff8723..f7bd229 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -2270,6 +2270,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>  \t\t\tgot_rev_arg = 1;\n>  \t}\n>  \n> +\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n> +\t    revs->diffopt.pathspec.nr == 1)\n> +\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n> +\n>  \tif (prune_data.nr) {\n>  \t\t/*\n>  \t\t * If we need to introduce the magic \"a lone ':' means no\n\nrevs->diffopt.pathspec isn't set up yet then. But prune_data is, so I\ncan use that. \n\nWill send a v3.\n"},{"id":"265801","messageId":"xmqqa8v71e4c.fsf@gitster.dls.corp.google.com","threadId":"39802","inReplyTo":"1436318939.5521.32.camel@twopensource.com","subject":"Re: [PATCH v2] log: add log.follow config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-08T04:12:51Z","receivedAt":"2015-07-08T04:12:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n>> IOW, I'd like to know why we need more than something like this\n>> change to this file, instead of the above?  We didn't muck with\n>> revs->diff in the original when FOLLOW_RENAMES was set, but now it\n>> does, for example.\n>\n> We did, but we did it earlier.  But I can just rearrange the code.\n\nAh, I see.  You don't have to move the existing code for that then.\nJust insert the \"if prune has one element and DEFAULT_ is set\" thing\nbefore the first use of FOLLOW_RENAMES (i.e. \"pick, filter and follow\nneeds diff\" piece) and you are done, I think.\n\nThanks.\n\n>\n>> diff --git a/revision.c b/revision.c\n>> index 3ff8723..f7bd229 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -2270,6 +2270,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>>  \t\t\tgot_rev_arg = 1;\n>>  \t}\n>>  \n>> +\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n>> +\t    revs->diffopt.pathspec.nr == 1)\n>> +\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n>> +\n>>  \tif (prune_data.nr) {\n>>  \t\t/*\n>>  \t\t * If we need to introduce the magic \"a lone ':' means no\n>\n> revs->diffopt.pathspec isn't set up yet then. But prune_data is, so I\n> can use that. \n>\n> Will send a v3.\n"},{"id":"265858","messageId":"xmqqzj36y4ql.fsf@gitster.dls.corp.google.com","threadId":"39802","inReplyTo":"xmqqsi8z1urz.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] log: add log.follow config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-08T16:49:06Z","receivedAt":"2015-07-08T16:49:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> David Turner <dturner@twopensource.com> writes:\n>\n>> diff --git a/revision.c b/revision.c\n>> index 3ff8723..ae6d4c3 100644\n>> --- a/revision.c\n>> +++ b/revision.c\n>> @@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>>  \n>>  \tif (revs->prune_data.nr) {\n>>  \t\tcopy_pathspec(&revs->pruning.pathspec, &revs->prune_data);\n>> -\t\t/* Can't prune commits with rename following: the paths change.. */\n>> -\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n>> -\t\t\trevs->prune = 1;\n>> +\n>>  \t\tif (!revs->full_diff)\n>>  \t\t\tcopy_pathspec(&revs->diffopt.pathspec,\n>>  \t\t\t\t      &revs->prune_data);\n>> +\n>> +\t\tif (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n>> +\t\t    revs->diffopt.pathspec.nr == 1)\n>> +\t\t\tDIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);\n>> +\n>> +\t\t/* Can't prune commits with rename following: the paths change.. */\n>> +\t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {\n>> +\t\t\trevs->prune = 1;\n>> +\t\t} else {\n>> +\t\t\trevs->diff = 1;\n>> +\t\t}\n>>  \t}\n>>  \tif (revs->combine_merges)\n>>  \t\trevs->ignore_merges = 0;\n>\n> It is unfortunate that we have to waste one DIFF_OPT bit and touch\n> revision.c for something that is \"just a convenience\".  Because\n> setup_revisions() does not have a way to let you flip the settings\n> depending on the number of pathspecs specified, I do not think of a\n> solution that does not have to touch a low level foundation part of\n> the codebase, which I'd really want to avoid.\n\nWhoa, wait.\n\nWhat is this opt->tweak thing doing here?\n\n    int setup_revisions(int argc, const char **argv, ...\n    {\n    ...\n            /* Second, deal with arguments and options */\n    ...\n            if (prune_data.nr) {\n    ...\n                    parse_pathspec(&revs->prune_data, 0, 0,\n                                   revs->prefix, prune_data.path);\n            }\n\n            if (revs->def == NULL)\n                    revs->def = opt ? opt->def : NULL;\n            if (opt && opt->tweak)\n                    opt->tweak(revs, opt);\n    ...\n            if (revs->prune_data.nr) {\n                    copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);\n                    /* Can't prune commits with rename following...\n                    if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n                            revs->prune = 1;\n                    if (!revs->full_diff)\n                            copy_pathspec(&revs->diffopt.pathspec,\n                                          &revs->prune_data);\n            }\n    ...\n    }\n\n\nIt seems that my earlier \"setup_revisions() does not have a way to\nlet you flip the settings depending on the number of pathspecs\nspecified\" is false.  We had to solve a similar issue when we did\nb4490059 (show -c: show patch text, 2010-03-08), and the mechanism\nfor doing such tweaking of the effect by command line options is\nalready there.\n\nNow I am wondering if the following suffices---instead of adding\nanother special case to setup_revisions(), we can tweak with an\nexisting mechanism.\n\nYou may want to rename show_rev_tweak_rev() and call the new\ndefault_follow_tweak() function at the end of that existing tweak\nfunction, if you want \"git show\" to also pay attention to the\nlog.follow configuration.  I don't think it is necessary.\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 6a068f7..d06248a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -625,6 +625,14 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n \treturn cmd_log_walk(&rev);\n }\n \n+static void default_follow_tweak(struct rev_info *rev,\n+\t\t\t\t struct setup_revision_opt *opt)\n+{\n+\tif (DIFF_OPT_TST(&rev->diffopt, DEFAULT_FOLLOW_RENAMES) &&\n+\t    rev->prune_data.nr == 1)\n+\t\tDIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);\n+}\n+\n int cmd_log(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info rev;\n@@ -638,6 +646,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = \"HEAD\";\n \topt.revarg_opt = REVARG_COMMITTISH;\n+\topt.tweak = default_follow_tweak;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n \treturn cmd_log_walk(&rev);\n }\n"}]}