 Documentation/diff-options.txt |    7 +++
 Makefile                       |    2 +
 diff.c                         |   16 ++++++
 diff.h                         |    1 +
 submodule.c                    |  112 ++++++++++++++++++++++++++++++++++++++++
 submodule.h                    |    8 +++
 6 files changed, 146 insertions(+), 0 deletions(-)
 create mode 100644 submodule.c
 create mode 100644 submodule.h

Documentation/diff-options.txt +7 -0

index 9276fae..99cb517 100644
@@ -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.
+
 --color::
 	Show colored diff.

Makefile +2 -0

index c0eff64..2f61e17 100644
@@ -453,6 +453,7 @@ LIB_H += sideband.h
 LIB_H += sigchain.h
 LIB_H += strbuf.h
 LIB_H += string-list.h
+LIB_H += submodule.h
 LIB_H += tag.h
 LIB_H += transport.h
 LIB_H += tree.h

@@ -551,6 +552,7 @@ LIB_OBJS += sideband.o
 LIB_OBJS += sigchain.o
 LIB_OBJS += strbuf.o
 LIB_OBJS += string-list.o
+LIB_OBJS += submodule.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
 LIB_OBJS += trace.o

diff.c +16 -0

index c719ce2..8af1ae2 100644
@@ -13,6 +13,7 @@
 #include "utf8.h"
 #include "userdiff.h"
 #include "sigchain.h"
+#include "submodule.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0

@@ -1557,6 +1558,17 @@ static void builtin_diff(const char *name_a,
 	const char *a_prefix, *b_prefix;
 	const char *textconv_one = NULL, *textconv_two = NULL;
 
+	if (DIFF_OPT_TST(o, SUBMODULE_LEFT_RIGHT_LOG) &&
+			(!one->mode || S_ISGITLINK(one->mode)) &&
+			(!two->mode || S_ISGITLINK(two->mode))) {
+		const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
+		const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
+		show_submodule_summary(o->file, one ? one->path : two->path,
+				one->sha1, two->sha1,
+				del, add, reset);
+		return;
+	}
+
 	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
 		textconv_one = get_textconv(one);
 		textconv_two = get_textconv(two);

@@ -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);
+	}
 
 	/* misc options */
 	else if (!strcmp(arg, "-z"))

diff.h +1 -0

index a7e7ccb..8079f5b 100644
@@ -67,6 +67,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
 #define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)
+#define DIFF_OPT_SUBMODULE_LEFT_RIGHT_LOG (1 << 23)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)

submodule.c new file +112 -0

112 lines not on this page · raw message

submodule.h new file +8 -0

new file mode 100644
index 0000000..4c0269d
@@ -0,0 +1,8 @@
+#ifndef SUBMODULE_H
+#define SUBMODULE_H
+
+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);
+
+#endif

Signature · 1 line

1.6.5.3.g464e1.dirty

Junio C Hamano

2016-06-15 22:47 UTC #1 lore raw

Quoted 5 lines · Jens Lehmann

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.

Quoted 8 lines

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 <[email protected]>
Signed-off-by Jens Lehmann <[email protected]>
---

Thanks.

I have four patches queued on js/diff-verbose-submodule topic and I think this corresponds to the first three, correct?

Quoted 5 lines

Documentation/diff-options.txt +0 -0
index 9276fae..99cb517 100644

Documentation/diff-options.txt endif::git-format-patch[] · line 96

Quoted 10 lines

Documentation/diff-options.txt +7 -0

@@ -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?

diff.c int diff_opt_parse(struct diff_options *options, const char **av, int ac) · line 2789

Quoted 12 lines

diff.c +4 -0

index c719ce2..8af1ae2 100644
@@ -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.

Quoted 12 lines

submodule.c new file +5 -0

new file mode 100644
index 0000000..206386f
@@ -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)
+{
...

submodule.c line 112

Quoted 10 lines

submodule.c new file +10 -0

+	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?

Jens Lehmann

author

2016-06-15 22:47 UTC #2 lore raw

Quoted 2 lines · Junio C Hamano

I have four patches queued on js/diff-verbose-submodule topic and I think this corresponds to the first three, correct?

Yes.

Quoted 6 lines

+--submodule[=<format>]::

Documentation/diff-options.txt +5 -0

+	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.

Quoted 8 lines

Quoted 1 line
+
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.

O.k., "--submodule=log" it is.

Quoted 2 lines

After all, if you do not give --submodule, we will give the traditional short format, no?

Yes. Will make the documentation clearer on this.

Quoted 7 lines

diff.c +0 -0

index c719ce2..8af1ae2 100644
@@ -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"))

Quoted 8 lines

diff.c int diff_opt_parse(struct diff_options *options, const char **av, int ac) · line 2789
Quoted 5 lines
diff.c +4 -0
 		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.

Will change that.

Quoted 16 lines

submodule.c new file +9 -0

new file mode 100644
index 0000000..206386f
@@ -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))) {

Quoted 8 lines

submodule.c line 112
Quoted 6 lines
+ ...
submodule.c new file +5 -0
+		}
+		clear_commit_marks(left, ~0);
+		clear_commit_marks(right, ~0);
+	}
+}
I thought we had strbuf_release(&sb) here... Where did it go?

*blush* ... thanks for catching this, will put it back where it belongs.

Jens Lehmann

author

2016-06-15 22:47 UTC #3 lore raw

PATCH v2 1/1 Add the --submodule option to the diff option family

When you use the option --submodule=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 <[email protected]>

Signed-off-by Jens Lehmann <[email protected]>

---

Changes since v1:
- Changed the option name from "left-right-log" to "log"
- Handling "--submodule"-option without extra arguments now
- Re-added a "strbuf_release(&sb);" being accidentally dropped
- Changed the wording in the Documentation

 Documentation/diff-options.txt |    7 +++
 Makefile                       |    2 +
 diff.c                         |   18 ++++++
 diff.h                         |    1 +
 submodule.c                    |  113 ++++++++++++++++++++++++++++++++++++++++
 submodule.h                    |    8 +++
 6 files changed, 149 insertions(+), 0 deletions(-)
 create mode 100644 submodule.c
 create mode 100644 submodule.h

Documentation/diff-options.txt +7 -0

index 9276fae..e26b847 100644
@@ -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 'log'. 'short' just shows pairs of commit names, this format
+	is used when this option is not given. 'log' is the default value for this
+	option and lists the commits in that commit range like the 'summary'
+	option of linkgit:git-submodule[1] does.
+
 --color::
 	Show colored diff.

Makefile +2 -0

index c0eff64..2f61e17 100644
@@ -453,6 +453,7 @@ LIB_H += sideband.h
 LIB_H += sigchain.h
 LIB_H += strbuf.h
 LIB_H += string-list.h
+LIB_H += submodule.h
 LIB_H += tag.h
 LIB_H += transport.h
 LIB_H += tree.h

@@ -551,6 +552,7 @@ LIB_OBJS += sideband.o
 LIB_OBJS += sigchain.o
 LIB_OBJS += strbuf.o
 LIB_OBJS += string-list.o
+LIB_OBJS += submodule.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
 LIB_OBJS += trace.o

diff.c +18 -0

index c719ce2..5fc9f37 100644
@@ -13,6 +13,7 @@
 #include "utf8.h"
 #include "userdiff.h"
 #include "sigchain.h"
+#include "submodule.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0

@@ -1557,6 +1558,17 @@ static void builtin_diff(const char *name_a,
 	const char *a_prefix, *b_prefix;
 	const char *textconv_one = NULL, *textconv_two = NULL;
 
+	if (DIFF_OPT_TST(o, SUBMODULE_LEFT_RIGHT_LOG) &&
+			(!one->mode || S_ISGITLINK(one->mode)) &&
+			(!two->mode || S_ISGITLINK(two->mode))) {
+		const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
+		const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
+		show_submodule_summary(o->file, one ? one->path : two->path,
+				one->sha1, two->sha1,
+				del, add, reset);
+		return;
+	}
+
 	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
 		textconv_one = get_textconv(one);
 		textconv_two = get_textconv(two);

@@ -2771,6 +2783,12 @@ 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 (!strcmp(arg, "--submodule"))
+		DIFF_OPT_SET(options, SUBMODULE_LEFT_RIGHT_LOG);
+	else if (!prefixcmp(arg, "--submodule=")) {
+		if (!strcmp(arg + 12, "log"))
+			DIFF_OPT_SET(options, SUBMODULE_LEFT_RIGHT_LOG);
+	}
 
 	/* misc options */
 	else if (!strcmp(arg, "-z"))

diff.h +1 -0

index a7e7ccb..8079f5b 100644
@@ -67,6 +67,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
 #define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)
+#define DIFF_OPT_SUBMODULE_LEFT_RIGHT_LOG (1 << 23)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)

submodule.c new file +113 -0

113 lines not on this page · raw message

submodule.h new file +8 -0

new file mode 100644
index 0000000..4c0269d
@@ -0,0 +1,8 @@
+#ifndef SUBMODULE_H
+#define SUBMODULE_H
+
+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);
+
+#endif

Signature · 1 line

1.6.5.104.gd732a