Re: [PATCH v4 2/2] diff: add SUBMODULE_DIFF format to display submodule diff
Jacob Keller writes:
> diff --git a/submodule.c b/submodule.c
> index 1b5cdfb7e784..36f8fd00c3ce 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -333,6 +333,73 @@ static void print_submodule_summary(struct rev_info
> *rev, FILE *f,
> strbuf_release(&sb);
> }
>
> +void show_submodule_diff(FILE *f, const char *path,
> + const char *line_prefix,
> + unsigned char one[20], unsigned char two[20],
> + unsigned dirty_submodule, const char *meta,
> + const char *a_prefix, const char *b_prefix,
> + const char *reset)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + struct strbuf submodule_git_dir = STRBUF_INIT;
> + const char *git_dir, *message = NULL;
> + int create_dirty_diff = 0;
> + FILE *diff;
> + char c;
The variables message, diff and c are not used, are they?
--
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
Re: [PATCH v4 2/2] diff: add SUBMODULE_DIFF format to display submodule diff
On Thu, Aug 11, 2016 at 10:53 AM, Junio C Hamano wrote: > Jacob Keller writes: > >> From: Jacob Keller >> >> 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 >> --- >> 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=" 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] diff: add SUBMODULE_DIFF format to display submodule diff
Jacob Keller writes: > From: Jacob Keller > > 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 > --- > 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=" 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". > 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. Thanks. -- 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
