On Thu, Aug 11, 2016 at 10:53 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jacob Keller <jacob.e.keller@xxxxxxxxx> writes: > >> From: Jacob Keller <jacob.keller@xxxxxxxxx> >> >> Teach git-diff and friends a new format for displaying the difference of >> a submodule using git-diff inside the submodule project. This allows >> users to easily see exactly what source changed in a given commit that >> updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit >> from the diff options, and instead add a new enum type for these >> formats. >> >> Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx> >> --- >> Documentation/diff-config.txt | 3 +- >> Documentation/diff-options.txt | 6 ++-- >> diff.c | 41 ++++++++++++++++---------- >> diff.h | 9 +++++- >> submodule.c | 67 ++++++++++++++++++++++++++++++++++++++++++ >> submodule.h | 6 ++++ >> 6 files changed, 113 insertions(+), 19 deletions(-) > > This looks good. > > You'd want some tests to make sure that the "--submodule" and the > "--submodule=<format>" command line options and the diff.submodule > configuration variables are parsed correctly (and when combined, the > command line option overrides the configured default), and of course > the machinery does the right thing, with and without "--graph" when > used in "git log". Yes, I am adding tests now, but I ran into some interesting corner cases for this, that still need some work. There's a bunch of issues with cases involving adding a submodule that isn't stored in .git/modules/etc, which the current tests for --submodule=log do. There's also the case of empty trees, which I believe I have resolved now. Hopefully I can sort these cases correctly. > >> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt >> index e7b729f3644f..988068225463 100644 >> --- a/Documentation/diff-options.txt >> +++ b/Documentation/diff-options.txt >> @@ -215,8 +215,10 @@ any of those replacements occurred. >> the commits in the range like linkgit:git-submodule[1] `summary` does. >> Omitting the `--submodule` option or specifying `--submodule=short`, >> uses the 'short' format. This format just shows the names of the commits >> - at the beginning and end of the range. Can be tweaked via the >> - `diff.submodule` configuration variable. >> + at the beginning and end of the range. When `--submodule=diff` is >> + given, the 'diff' format is used. This format shows the diff between >> + the old and new submodule commmit from the perspective of the >> + submodule. Can be tweaked via the `diff.submodule` configuration variable. > > This is carried over from the existing text, but "Can be tweaked > via" sounds a bit wasteful (and strange); "Defaults to" (or "The > default is taken from") is the phrase we seem to use more often. Probably worth fixing here. WIll do. Thanks, Jake > > Thanks. -- 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