Skip to content

Commit fd47ae6

Browse files
jacob-kellergitster
jacob-keller
authored and
gitster
committed
diff: teach diff to display submodule difference with an inline diff
Teach git-diff and friends a new format for displaying the difference of a submodule. The new format is an inline diff of the contents of the submodule between the commit range of the update. This allows the user to see the actual code change caused by a submodule update. Add tests for the new format and option. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 8e6df65 commit fd47ae6

File tree

7 files changed

+863
-21
lines changed

7 files changed

+863
-21
lines changed

‎Documentation/diff-config.txt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,11 @@ diff.suppressBlankEmpty::
122122

123123
diff.submodule::
124124
Specify the format in which differences in submodules are
125-
shown. The "log" format lists the commits in the range like
126-
linkgit:git-submodule[1] `summary` does. The "short" format
127-
format just shows the names of the commits at the beginning
128-
and end of the range. Defaults to short.
125+
shown. The "short" format just shows the names of the commits
126+
at the beginning and end of the range. The "log" format lists
127+
the commits in the range like linkgit:git-submodule[1] `summary`
128+
does. The "diff" format shows an inline diff of the changed
129+
contents of the submodule. Defaults to "short".
129130

130131
diff.wordRegex::
131132
A POSIX Extended Regular Expression used to determine what is a "word"

‎Documentation/diff-options.txt‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,16 @@ any of those replacements occurred.
210210
of the `--diff-filter` option on what the status letters mean.
211211

212212
--submodule[=<format>]::
213-
Specify how differences in submodules are shown. When `--submodule`
214-
or `--submodule=log` is given, the 'log' format is used. This format lists
215-
the commits in the range like linkgit:git-submodule[1] `summary` does.
216-
Omitting the `--submodule` option or specifying `--submodule=short`,
217-
uses the 'short' format. This format just shows the names of the commits
218-
at the beginning and end of the range. Can be tweaked via the
219-
`diff.submodule` configuration variable.
213+
Specify how differences in submodules are shown. When specifying
214+
`--submodule=short` the 'short' format is used. This format just
215+
shows the names of the commits at the beginning and end of the range.
216+
When `--submodule` or `--submodule=log` is specified, the 'log'
217+
format is used. This format lists the commits in the range like
218+
linkgit:git-submodule[1] `summary` does. When `--submodule=diff`
219+
is specified, the 'diff' format is used. This format shows an
220+
inline diff of the changes in the submodule contents between the
221+
commit range. Defaults to `diff.submodule` or the 'short' format
222+
if the config option is unset.
220223

221224
--color[=<when>]::
222225
Show colored diff.

‎diff.c‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ static int parse_submodule_params(struct diff_options *options, const char *valu
135135
options->submodule_format = DIFF_SUBMODULE_LOG;
136136
else if (!strcmp(value, "short"))
137137
options->submodule_format = DIFF_SUBMODULE_SHORT;
138+
else if (!strcmp(value, "diff"))
139+
options->submodule_format = DIFF_SUBMODULE_INLINE_DIFF;
138140
else
139141
return -1;
140142
return 0;
@@ -2300,6 +2302,15 @@ static void builtin_diff(const char *name_a,
23002302
struct strbuf header = STRBUF_INIT;
23012303
const char *line_prefix = diff_line_prefix(o);
23022304

2305+
diff_set_mnemonic_prefix(o, "a/", "b/");
2306+
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
2307+
a_prefix = o->b_prefix;
2308+
b_prefix = o->a_prefix;
2309+
} else {
2310+
a_prefix = o->a_prefix;
2311+
b_prefix = o->b_prefix;
2312+
}
2313+
23032314
if (o->submodule_format == DIFF_SUBMODULE_LOG &&
23042315
(!one->mode || S_ISGITLINK(one->mode)) &&
23052316
(!two->mode || S_ISGITLINK(two->mode))) {
@@ -2311,22 +2322,24 @@ static void builtin_diff(const char *name_a,
23112322
two->dirty_submodule,
23122323
meta, del, add, reset);
23132324
return;
2325+
} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
2326+
(!one->mode || S_ISGITLINK(one->mode)) &&
2327+
(!two->mode || S_ISGITLINK(two->mode))) {
2328+
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
2329+
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
2330+
show_submodule_inline_diff(o->file, one->path ? one->path : two->path,
2331+
line_prefix,
2332+
&one->oid, &two->oid,
2333+
two->dirty_submodule,
2334+
meta, del, add, reset, o);
2335+
return;
23142336
}
23152337

23162338
if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
23172339
textconv_one = get_textconv(one);
23182340
textconv_two = get_textconv(two);
23192341
}
23202342

2321-
diff_set_mnemonic_prefix(o, "a/", "b/");
2322-
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
2323-
a_prefix = o->b_prefix;
2324-
b_prefix = o->a_prefix;
2325-
} else {
2326-
a_prefix = o->a_prefix;
2327-
b_prefix = o->b_prefix;
2328-
}
2329-
23302343
/* Never use a non-valid filename anywhere if at all possible */
23312344
name_a = DIFF_FILE_VALID(one) ? name_a : name_b;
23322345
name_b = DIFF_FILE_VALID(two) ? name_b : name_a;

‎diff.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ enum diff_words_type {
111111

112112
enum diff_submodule_format {
113113
DIFF_SUBMODULE_SHORT = 0,
114-
DIFF_SUBMODULE_LOG
114+
DIFF_SUBMODULE_LOG,
115+
DIFF_SUBMODULE_INLINE_DIFF
115116
};
116117

117118
struct diff_options {

‎submodule.c‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,75 @@ void show_submodule_summary(FILE *f, const char *path,
442442
clear_commit_marks(right, ~0);
443443
}
444444

445+
void show_submodule_inline_diff(FILE *f, const char *path,
446+
const char *line_prefix,
447+
struct object_id *one, struct object_id *two,
448+
unsigned dirty_submodule, const char *meta,
449+
const char *del, const char *add, const char *reset,
450+
const struct diff_options *o)
451+
{
452+
const struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;
453+
struct commit *left = NULL, *right = NULL;
454+
struct commit_list *merge_bases = NULL;
455+
struct strbuf submodule_dir = STRBUF_INIT;
456+
struct child_process cp = CHILD_PROCESS_INIT;
457+
458+
show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
459+
meta, reset, &left, &right, &merge_bases);
460+
461+
/* We need a valid left and right commit to display a difference */
462+
if (!(left || is_null_oid(one)) ||
463+
!(right || is_null_oid(two)))
464+
goto done;
465+
466+
if (left)
467+
old = one;
468+
if (right)
469+
new = two;
470+
471+
fflush(f);
472+
cp.git_cmd = 1;
473+
cp.dir = path;
474+
cp.out = dup(fileno(f));
475+
cp.no_stdin = 1;
476+
477+
/* TODO: other options may need to be passed here. */
478+
argv_array_push(&cp.args, "diff");
479+
argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
480+
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
481+
argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
482+
o->b_prefix, path);
483+
argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",
484+
o->a_prefix, path);
485+
} else {
486+
argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
487+
o->a_prefix, path);
488+
argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",
489+
o->b_prefix, path);
490+
}
491+
argv_array_push(&cp.args, oid_to_hex(old));
492+
/*
493+
* If the submodule has modified content, we will diff against the
494+
* work tree, under the assumption that the user has asked for the
495+
* diff format and wishes to actually see all differences even if they
496+
* haven't yet been committed to the submodule yet.
497+
*/
498+
if (!(dirty_submodule & DIRTY_SUBMODULE_MODIFIED))
499+
argv_array_push(&cp.args, oid_to_hex(new));
500+
501+
if (run_command(&cp))
502+
fprintf(f, "(diff failed)\n");
503+
504+
done:
505+
strbuf_release(&submodule_dir);
506+
if (merge_bases)
507+
free_commit_list(merge_bases);
508+
if (left)
509+
clear_commit_marks(left, ~0);
510+
if (right)
511+
clear_commit_marks(right, ~0);
512+
}
513+
445514
void set_config_fetch_recurse_submodules(int value)
446515
{
447516
config_fetch_recurse_submodules = value;

‎submodule.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ void show_submodule_summary(FILE *f, const char *path,
4646
struct object_id *one, struct object_id *two,
4747
unsigned dirty_submodule, const char *meta,
4848
const char *del, const char *add, const char *reset);
49+
void show_submodule_inline_diff(FILE *f, const char *path,
50+
const char *line_prefix,
51+
struct object_id *one, struct object_id *two,
52+
unsigned dirty_submodule, const char *meta,
53+
const char *del, const char *add, const char *reset,
54+
const struct diff_options *opt);
4955
void set_config_fetch_recurse_submodules(int value);
5056
void check_for_new_submodule_commits(unsigned char new_sha1[20]);
5157
int fetch_populated_submodules(const struct argv_array *options,

0 commit comments

Comments
 (0)