Robert Dailey <[email protected]> writes:
> I could have gone through the effort to make this more configurable, but
> before doing that level of work I wanted to get some discussion going to
> understand first if this is a useful change and second how it should be
> configured. For example, we could allow:
>
> $ git diff --submodule=long-log
>
> Or a supplementary option such as:
>
> $ git diff --submodule=log --submodule-log-detail=(long|short)
>
> I'm not sure what makes sense here. I welcome thoughts/discussion and
> will provide follow-up patches.
My quick looking around reveals that prepare_submodule_summary() is
called only by show_submodule_summary(), which in turn is called
only from builtin_diff() in a codepath like this:
if (o->submodule_format == DIFF_SUBMODULE_LOG &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
show_submodule_summary(o, one->path ? one->path : two->path,
&one->oid, &two->oid,
two->dirty_submodule);
return;
} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
show_submodule_inline_diff(o, one->path ? one->path : two->path,
&one->oid, &two->oid,
two->dirty_submodule);
return;
}
It looks like introducing a new value to o->submodule_format (enum
diff_submodule_format defined in diff.h) would be one natural way to
extend this codepath, at least to me from a quick glance.
It also looks to me that the above may become far easier to read if
the common "are we dealing with a filepair <one, two> that involves
submodules?" check in the above if/else if cascade is factored out,
perhaps like this as a preliminary clean-up step, before adding a
new value:
if ((!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
switch (o->submodule_format) {
case DIFF_SUBMODULE_LOG:
... do the "log" thing ...
return;
case DIFF_SUBMODULE_INLINE_DIFF:
... do the "inline" thing ...
return;
default:
break;
}
}
Then the place to add a new format would be trivially obvious,
i.e. just add a new case arm to call a new function to give the
summary.
