Junio C Hamano <gits...@pobox.com> writes:

> David Turner <dtur...@twopensource.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to