Re: [PATCH] Add the --submodule option to the diff option family
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 19, 2009, 03:02 UTC
- Message-ID
- <7v3a5gdr1c.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <4ADA10FC.40708@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 13 quoted lines
> When you use the option --submodule=left-right-log you can see the submodule > summaries inlined in the diff, instead of not-quite-helpful SHA-1 pairs. > > The format imitates what "git submodule summary" shows. > > To do that, <path>/.git/objects/ is added to the alternate object > databases (if that directory exists). > > This option was requested by Jens Lehmann at the GitTogether in Berlin. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> > ---
Thanks.
I have four patches queued on js/diff-verbose-submodule topic and I think this corresponds to the first three, correct?
Show 15 quoted lines
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt > index 9276fae..99cb517 100644 > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -87,6 +87,13 @@ endif::git-format-patch[] > Show only names and status of changed files. See the description > of the `--diff-filter` option on what the status letters mean. > > +--submodule[=<format>]:: > + Chose the output format for submodule differences. <format> can be one of > + 'short' and 'left-right-log'. 'short' is the default value for this > + option and and shows pairs of commit names. 'left-right-log' lists the > + commits in that commit range like the 'summary' option of > + linkgit:git-submodule[1] does. > +
Well, while left-right-log may be logically the most correct name for this option, I think it is too long to be practical. Because it is not like we would want to have an option to have full log there when we are showing "diff", I think it would make sense to making left-right-log the default when "--submodule" (without format specification) is given, and possibly give "--submodule=log" as the synonym for this format.
After all, if you do not give --submodule, we will give the traditional short format, no?
Show 12 quoted lines
> diff --git a/diff.c b/diff.c
> index c719ce2..8af1ae2 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2771,6 +2783,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
> DIFF_OPT_CLR(options, ALLOW_TEXTCONV);
> else if (!strcmp(arg, "--ignore-submodules"))
> DIFF_OPT_SET(options, IGNORE_SUBMODULES);
> + else if (!prefixcmp(arg, "--submodule=")) {
> + if (!strcmp(arg + 12, "left-right-log"))
> + DIFF_OPT_SET(options, SUBMODULE_LEFT_RIGHT_LOG);
> + }I do not see --submodule (without "=<format>") supported here as the documentation claims, but this is the logical place to do so.
Show 22 quoted lines
> diff --git a/submodule.c b/submodule.c
> new file mode 100644
> index 0000000..206386f
> --- /dev/null
> +++ b/submodule.c
> @@ -0,0 +1,112 @@
> +...
> +void show_submodule_summary(FILE *f, const char *path,
> + unsigned char one[20], unsigned char two[20],
> + const char *del, const char *add, const char *reset)
> +{
> ...
> + fwrite(sb.buf, sb.len, 1, f);
> +
> + if (!message) {
> + while ((commit = get_revision(&rev))) {
> + ...
> + }
> + clear_commit_marks(left, ~0);
> + clear_commit_marks(right, ~0);
> + }
> +}I thought we had strbuf_release(&sb) here... Where did it go?