Junio C Hamano <gitster@xxxxxxxxx> writes:
> David Turner <dturner@xxxxxxxxxxxxxxxx> writes:
>
>> diff --git a/revision.c b/revision.c
>> index 3ff8723..ae6d4c3 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -2322,12 +2322,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>>
>> if (revs->prune_data.nr) {
>> copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
>> - /* Can't prune commits with rename following: the paths change.. */
>> - if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
>> - revs->prune = 1;
>> +
>> if (!revs->full_diff)
>> copy_pathspec(&revs->diffopt.pathspec,
>> &revs->prune_data);
>> +
>> + if (DIFF_OPT_TST(&revs->diffopt, DEFAULT_FOLLOW_RENAMES) &&
>> + revs->diffopt.pathspec.nr == 1)
>> + DIFF_OPT_SET(&revs->diffopt, FOLLOW_RENAMES);
>> +
>> + /* Can't prune commits with rename following: the paths change.. */
>> + if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES)) {
>> + revs->prune = 1;
>> + } else {
>> + revs->diff = 1;
>> + }
>> }
>> if (revs->combine_merges)
>> revs->ignore_merges = 0;
>
> It is unfortunate that we have to waste one DIFF_OPT bit and touch
> revision.c for something that is "just a convenience". Because
> setup_revisions() does not have a way to let you flip the settings
> depending on the number of pathspecs specified, I do not think of a
> solution that does not have to touch a low level foundation part of
> the codebase, which I'd really want to avoid.
Whoa, wait.
What is this opt->tweak thing doing here?
int setup_revisions(int argc, const char **argv, ...
{
...
/* Second, deal with arguments and options */
...
if (prune_data.nr) {
...
parse_pathspec(&revs->prune_data, 0, 0,
revs->prefix, prune_data.path);
}
if (revs->def == NULL)
revs->def = opt ? opt->def : NULL;
if (opt && opt->tweak)
opt->tweak(revs, opt);
...
if (revs->prune_data.nr) {
copy_pathspec(&revs->pruning.pathspec, &revs->prune_data);
/* Can't prune commits with rename following...
if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
revs->prune = 1;
if (!revs->full_diff)
copy_pathspec(&revs->diffopt.pathspec,
&revs->prune_data);
}
...
}
It seems that my earlier "setup_revisions() does not have a way to
let you flip the settings depending on the number of pathspecs
specified" is false. We had to solve a similar issue when we did
b4490059 (show -c: show patch text, 2010-03-08), and the mechanism
for doing such tweaking of the effect by command line options is
already there.
Now I am wondering if the following suffices---instead of adding
another special case to setup_revisions(), we can tweak with an
existing mechanism.
You may want to rename show_rev_tweak_rev() and call the new
default_follow_tweak() function at the end of that existing tweak
function, if you want "git show" to also pay attention to the
log.follow configuration. I don't think it is necessary.
diff --git a/builtin/log.c b/builtin/log.c
index 6a068f7..d06248a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -625,6 +625,14 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
return cmd_log_walk(&rev);
}
+static void default_follow_tweak(struct rev_info *rev,
+ struct setup_revision_opt *opt)
+{
+ if (DIFF_OPT_TST(&rev->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+ rev->prune_data.nr == 1)
+ DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);
+}
+
int cmd_log(int argc, const char **argv, const char *prefix)
{
struct rev_info rev;
@@ -638,6 +646,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)
memset(&opt, 0, sizeof(opt));
opt.def = "HEAD";
opt.revarg_opt = REVARG_COMMITTISH;
+ opt.tweak = default_follow_tweak;
cmd_log_init(argc, argv, prefix, &rev, &opt);
return cmd_log_walk(&rev);
}
--
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