* [PATCH v9 0/8] submodule show inline diff
@ 2016-08-19 23:34 Jacob Keller
2016-08-19 23:34 ` [PATCH v9 1/8] cache: add empty_tree_oid object and helper function Jacob Keller
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
More suggestions from Junio and a few changes to support submodule name
lookup. Hopefully we're getting close to the goal!
interdiff between v8 and current:
diff --git c/builtin/rev-list.c w/builtin/rev-list.c
index 21cde8dd6b31..8479f6ed28aa 100644
--- c/builtin/rev-list.c
+++ w/builtin/rev-list.c
@@ -129,29 +129,29 @@ static void show_commit(struct commit *commit, void *data)
graph_show_commit_msg(revs->graph, stdout, &buf);
/*
- * Add a newline after the commit message.
- *
- * Usually, this newline produces a blank
- * padding line between entries, in which case
- * we need to add graph padding on this line.
- *
- * However, the commit message may not end in a
- * newline. In this case the newline simply
- * ends the last line of the commit message,
- * and we don't need any graph output. (This
- * always happens with CMIT_FMT_ONELINE, and it
- * happens with CMIT_FMT_USERFORMAT when the
- * format doesn't explicitly end in a newline.)
- */
+ * Add a newline after the commit message.
+ *
+ * Usually, this newline produces a blank
+ * padding line between entries, in which case
+ * we need to add graph padding on this line.
+ *
+ * However, the commit message may not end in a
+ * newline. In this case the newline simply
+ * ends the last line of the commit message,
+ * and we don't need any graph output. (This
+ * always happens with CMIT_FMT_ONELINE, and it
+ * happens with CMIT_FMT_USERFORMAT when the
+ * format doesn't explicitly end in a newline.)
+ */
if (buf.len && buf.buf[buf.len - 1] == '\n')
graph_show_padding(revs->graph);
putchar('\n');
} else {
/*
- * If the message buffer is empty, just show
- * the rest of the graph output for this
- * commit.
- */
+ * If the message buffer is empty, just show
+ * the rest of the graph output for this
+ * commit.
+ */
if (graph_show_remainder(revs->graph))
putchar('\n');
if (revs->commit_format == CMIT_FMT_ONELINE)
diff --git c/cache.h w/cache.h
index da9f0be67d7b..70428e92d7ed 100644
--- c/cache.h
+++ w/cache.h
@@ -953,24 +953,39 @@ static inline void oidclr(struct object_id *oid)
#define EMPTY_TREE_SHA1_BIN_LITERAL \
"\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
"\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
-#define EMPTY_TREE_SHA1_BIN \
- ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)
+extern const struct object_id empty_tree_oid;
+#define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)
#define EMPTY_BLOB_SHA1_HEX \
"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"
#define EMPTY_BLOB_SHA1_BIN_LITERAL \
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
-#define EMPTY_BLOB_SHA1_BIN \
- ((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)
+extern const struct object_id empty_blob_oid;
+#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
-extern const struct object_id empty_tree_oid;
static inline int is_empty_blob_sha1(const unsigned char *sha1)
{
return !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);
}
+static inline int is_empty_blob_oid(const struct object_id *oid)
+{
+ return !hashcmp(oid->hash, EMPTY_BLOB_SHA1_BIN);
+}
+
+static inline int is_empty_tree_sha1(const unsigned char *sha1)
+{
+ return !hashcmp(sha1, EMPTY_TREE_SHA1_BIN);
+}
+
+static inline int is_empty_tree_oid(const struct object_id *oid)
+{
+ return !hashcmp(oid->hash, EMPTY_TREE_SHA1_BIN);
+}
+
+
int git_mkstemp(char *path, size_t n, const char *template);
/* set default permissions by passing mode arguments to open(2) */
diff --git c/diff.h w/diff.h
index 192c0eedd0ff..ec76a90522ea 100644
--- c/diff.h
+++ w/diff.h
@@ -112,7 +112,7 @@ enum diff_words_type {
enum diff_submodule_format {
DIFF_SUBMODULE_SHORT = 0,
DIFF_SUBMODULE_LOG,
- DIFF_SUBMODULE_INLINE_DIFF,
+ DIFF_SUBMODULE_INLINE_DIFF
};
struct diff_options {
diff --git c/path.c w/path.c
index 188abfebbafe..081a22c1163c 100644
--- c/path.c
+++ w/path.c
@@ -6,6 +6,7 @@
#include "string-list.h"
#include "dir.h"
#include "worktree.h"
+#include "submodule-config.h"
static int get_st_mode_bits(const char *path, int *mode)
{
@@ -474,6 +475,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
const char *git_dir;
struct strbuf git_submodule_common_dir = STRBUF_INIT;
struct strbuf git_submodule_dir = STRBUF_INIT;
+ const struct submodule *submodule_config;
strbuf_addstr(buf, path);
strbuf_complete(buf, '/');
@@ -486,7 +488,16 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
}
if (!is_git_directory(buf->buf)) {
strbuf_reset(buf);
- strbuf_git_path(buf, "%s/%s", "modules", path);
+ /*
+ * Lookup the submodule name from the config. If that fails
+ * fall back to assuming the path is the name.
+ */
+ submodule_config = submodule_from_path(null_sha1, path);
+ if (submodule_config)
+ strbuf_git_path(buf, "%s/%s", "modules",
+ submodule_config->name);
+ else
+ strbuf_git_path(buf, "%s/%s", "modules", path);
}
strbuf_addch(buf, '/');
diff --git c/sha1_file.c w/sha1_file.c
index 10883d56a600..21cf923bcf1f 100644
--- c/sha1_file.c
+++ w/sha1_file.c
@@ -39,7 +39,10 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
const unsigned char null_sha1[20];
const struct object_id null_oid;
const struct object_id empty_tree_oid = {
- .hash = EMPTY_TREE_SHA1_BIN_LITERAL
+ EMPTY_TREE_SHA1_BIN_LITERAL
+};
+const struct object_id empty_blob_oid = {
+ EMPTY_BLOB_SHA1_BIN_LITERAL
};
/*
diff --git c/submodule.c w/submodule.c
index cecd3cd98de4..405fa9e4eb32 100644
--- c/submodule.c
+++ w/submodule.c
@@ -277,9 +277,10 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
}
static int prepare_submodule_summary(struct rev_info *rev, const char *path,
- struct commit *left, struct commit *right)
+ struct commit *left, struct commit *right,
+ struct commit_list *merge_bases)
{
- struct commit_list *merge_bases, *list;
+ struct commit_list *list;
init_revisions(rev, NULL);
setup_revisions(0, NULL, rev, NULL);
@@ -288,7 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
left->object.flags |= SYMMETRIC_LEFT;
add_pending_object(rev, &left->object, path);
add_pending_object(rev, &right->object, path);
- merge_bases = get_merge_bases(left, right);
for (list = merge_bases; list; list = list->next) {
list->item->object.flags |= UNINTERESTING;
add_pending_object(rev, &list->item->object,
@@ -336,9 +336,9 @@ static void show_submodule_header(FILE *f, const char *path,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *reset,
- struct commit **left, struct commit **right)
+ struct commit **left, struct commit **right,
+ struct commit_list **merge_bases)
{
- struct commit_list *merge_bases;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
@@ -376,11 +376,11 @@ static void show_submodule_header(FILE *f, const char *path,
(!is_null_oid(two) && !*right))
message = "(commits not present)";
- merge_bases = get_merge_bases(*left, *right);
- if (merge_bases) {
- if (merge_bases->item == *left)
+ *merge_bases = get_merge_bases(*left, *right);
+ if (*merge_bases) {
+ if ((*merge_bases)->item == *left)
fast_forward = 1;
- else if (merge_bases->item == *right)
+ else if ((*merge_bases)->item == *right)
fast_backward = 1;
}
@@ -412,9 +412,10 @@ void show_submodule_summary(FILE *f, const char *path,
{
struct rev_info rev;
struct commit *left = NULL, *right = NULL;
+ struct commit_list *merge_bases = NULL;
show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
- meta, reset, &left, &right);
+ meta, reset, &left, &right, &merge_bases);
/*
* If we don't have both a left and a right pointer, there is no
@@ -422,15 +423,19 @@ void show_submodule_summary(FILE *f, const char *path,
* all the information the user needs.
*/
if (!left || !right)
- return;
+ goto out;
/* Treat revision walker failure the same as missing commits */
- if (prepare_submodule_summary(&rev, path, left, right)) {
+ if (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {
fprintf(f, "%s(revision walker failed)\n", line_prefix);
- return;
+ goto out;
}
print_submodule_summary(&rev, f, line_prefix, del, add, reset);
+
+out:
+ if (merge_bases)
+ free_commit_list(merge_bases);
clear_commit_marks(left, ~0);
clear_commit_marks(right, ~0);
}
@@ -444,11 +449,12 @@ void show_submodule_inline_diff(FILE *f, const char *path,
{
const struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;
struct commit *left = NULL, *right = NULL;
+ struct commit_list *merge_bases = NULL;
struct strbuf submodule_dir = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
- meta, reset, &left, &right);
+ meta, reset, &left, &right, &merge_bases);
/* We need a valid left and right commit to display a difference */
if (!(left || is_null_oid(one)) ||
@@ -467,7 +473,7 @@ void show_submodule_inline_diff(FILE *f, const char *path,
cp.no_stdin = 1;
/* TODO: other options may need to be passed here. */
- argv_array_pushl(&cp.args, "diff");
+ argv_array_push(&cp.args, "diff");
argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
@@ -495,6 +501,12 @@ void show_submodule_inline_diff(FILE *f, const char *path,
done:
strbuf_release(&submodule_dir);
+ if (merge_bases)
+ free_commit_list(merge_bases);
+ if (left)
+ clear_commit_marks(left, ~0);
+ if (right)
+ clear_commit_marks(right, ~0);
}
void set_config_fetch_recurse_submodules(int value)
diff --git c/t/t4059-diff-submodule-not-initialized.sh w/t/t4059-diff-submodule-not-initialized.sh
index cc787454033a..c8775854d3c2 100755
--- c/t/t4059-diff-submodule-not-initialized.sh
+++ w/t/t4059-diff-submodule-not-initialized.sh
@@ -9,7 +9,6 @@ This test tries to verify that add_submodule_odb works when the submodule was
initialized previously but the checkout has since been removed.
'
-TEST_NO_CREATE_REPO=1
. ./test-lib.sh
# Tested non-UTF-8 encoding
@@ -18,6 +17,7 @@ test_encoding="ISO8859-1"
# String "added" in German (translated with Google Translate), encoded in UTF-8,
# used in sample commit log messages in add_file() function below.
added=$(printf "hinzugef\303\274gt")
+
add_file () {
(
cd "$1" &&
@@ -35,44 +35,45 @@ add_file () {
git rev-parse --short --verify HEAD
)
}
+
commit_file () {
test_tick &&
git commit "$@" -m "Commit $*" >/dev/null
}
-test_create_repo sm1 &&
-test_create_repo sm2 &&
-add_file sm1 foo >/dev/null &&
-add_file sm2 foo1 foo2 >/dev/null &&
-smhead1=$(cd sm2; git rev-parse --short --verify HEAD) &&
-cd sm1
+test_expect_success 'setup - submodules' '
+ test_create_repo sm2 &&
+ add_file . foo &&
+ add_file sm2 foo1 foo2 &&
+ smhead1=$(git -C sm2 rev-parse --short --verify HEAD)
+'
-test_expect_success 'setup - submodule directory' '
- git submodule add ../sm2 sm2 &&
- commit_file sm2 &&
+test_expect_success 'setup - git submodule add' '
+ git submodule add ./sm2 sm1 &&
+ commit_file sm1 &&
git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm2 0000000...$smhead1 (new submodule)
+ Submodule sm1 0000000...$smhead1 (new submodule)
EOF
test_cmp expected actual
'
test_expect_success 'submodule directory removed' '
- rm -rf sm2 &&
+ rm -rf sm1 &&
git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm2 0000000...$smhead1 (new submodule)
+ Submodule sm1 0000000...$smhead1 (new submodule)
EOF
test_cmp expected actual
'
test_expect_success 'setup - submodule multiple commits' '
- git submodule update --checkout sm2 &&
- smhead2=$(add_file sm2 foo3 foo4) &&
- commit_file sm2 &&
+ git submodule update --checkout sm1 &&
+ smhead2=$(add_file sm1 foo3 foo4) &&
+ commit_file sm1 &&
git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm2 $smhead1..$smhead2:
+ Submodule sm1 $smhead1..$smhead2:
> Add foo4 ($added foo4)
> Add foo3 ($added foo3)
EOF
@@ -80,26 +81,47 @@ test_expect_success 'setup - submodule multiple commits' '
'
test_expect_success 'submodule removed multiple commits' '
- rm -rf sm2 &&
+ rm -rf sm1 &&
git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm2 $smhead1..$smhead2:
+ Submodule sm1 $smhead1..$smhead2:
> Add foo4 ($added foo4)
> Add foo3 ($added foo3)
EOF
test_cmp expected actual
'
-cd ..
-
test_expect_success 'submodule not initialized in new clone' '
- git clone sm1 sm3 &&
+ git clone . sm3 &&
git -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm2 $smhead1...$smhead2 (not initialized)
+ Submodule sm1 $smhead1...$smhead2 (not initialized)
EOF
test_cmp expected actual
+'
+test_expect_success 'setup submodule moved' '
+ git submodule update --checkout sm1 &&
+ git mv sm1 sm4 &&
+ commit_file sm4 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm4 0000000...$smhead2 (new submodule)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule moved then removed' '
+ smhead3=$(add_file sm4 foo6 foo7) &&
+ commit_file sm4 &&
+ rm -rf sm4 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm4 $smhead2..$smhead3:
+ > Add foo7 ($added foo7)
+ > Add foo6 ($added foo6)
+ EOF
+ test_cmp expected actual
'
test_done
diff --git c/t/t4060-diff-submodule-option-diff-format.sh w/t/t4060-diff-submodule-option-diff-format.sh
index 836caef5c8ba..7e23b55ea4c5 100755
--- c/t/t4060-diff-submodule-option-diff-format.sh
+++ w/t/t4060-diff-submodule-option-diff-format.sh
@@ -18,6 +18,7 @@ test_encoding="ISO8859-1"
# String "added" in German (translated with Google Translate), encoded in UTF-8,
# used in sample commit log messages in add_file() function below.
added=$(printf "hinzugef\303\274gt")
+
add_file () {
(
cd "$1" &&
@@ -35,16 +36,18 @@ add_file () {
git rev-parse --short --verify HEAD
)
}
+
commit_file () {
test_tick &&
git commit "$@" -m "Commit $*" >/dev/null
}
-test_create_repo sm1 &&
-add_file . foo >/dev/null
-
-head1=$(add_file sm1 foo1 foo2)
-fullhead1=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup repository' '
+ test_create_repo sm1 &&
+ add_file . foo &&
+ head1=$(add_file sm1 foo1 foo2) &&
+ fullhead1=$(git -C sm1 rev-parse --verify HEAD)
+'
test_expect_success 'added submodule' '
git add sm1 &&
-------->8
Jacob Keller (7):
cache: add empty_tree_oid object and helper function
graph: add support for --line-prefix on all graph-aware output
diff: prepare for additional submodule formats
submodule: allow add_submodule_odb to work even if path is not checked
out
submodule: convert show_submodule_summary to use struct object_id *
submodule: refactor show_submodule_summary with helper function
diff: teach diff to display submodule difference with an inline diff
Junio C Hamano (1):
diff.c: remove output_prefix_length field
Documentation/diff-config.txt | 9 +-
Documentation/diff-options.txt | 20 +-
builtin/rev-list.c | 70 +-
cache.h | 25 +-
diff.c | 64 +-
diff.h | 11 +-
graph.c | 100 ++-
graph.h | 22 +-
log-tree.c | 5 +-
path.c | 16 +
sha1_file.c | 6 +
submodule.c | 186 ++++-
submodule.h | 8 +-
t/t4013-diff-various.sh | 6 +
...diff.diff_--line-prefix=abc_master_master^_side | 29 +
t/t4013/diff.diff_--line-prefix_--cached_--_file0 | 15 +
t/t4059-diff-submodule-not-initialized.sh | 127 ++++
t/t4060-diff-submodule-option-diff-format.sh | 749 +++++++++++++++++++++
t/t4202-log.sh | 323 +++++++++
19 files changed, 1637 insertions(+), 154 deletions(-)
create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
create mode 100755 t/t4059-diff-submodule-not-initialized.sh
create mode 100755 t/t4060-diff-submodule-option-diff-format.sh
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 1/8] cache: add empty_tree_oid object and helper function
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 2/8] diff.c: remove output_prefix_length field Jacob Keller
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Similar to is_null_oid(), and is_empty_blob_sha1() add an
empty_tree_oid along with helper function is_empty_tree_oid(). For
completeness, also add an "is_empty_tree_sha1()",
"is_empty_blob_sha1()", "is_empty_tree_oid()" and "is_empty_blob_oid()"
helpers.
To ensure we only get one singleton, implement EMPTY_BLOB_SHA1_BIN as
simply getting the hash of empty_blob_oid structure.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
cache.h | 25 +++++++++++++++++++++----
sha1_file.c | 6 ++++++
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index f30a4417efdf..70428e92d7ed 100644
--- a/cache.h
+++ b/cache.h
@@ -953,22 +953,39 @@ static inline void oidclr(struct object_id *oid)
#define EMPTY_TREE_SHA1_BIN_LITERAL \
"\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
"\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
-#define EMPTY_TREE_SHA1_BIN \
- ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)
+extern const struct object_id empty_tree_oid;
+#define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)
#define EMPTY_BLOB_SHA1_HEX \
"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"
#define EMPTY_BLOB_SHA1_BIN_LITERAL \
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
-#define EMPTY_BLOB_SHA1_BIN \
- ((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)
+extern const struct object_id empty_blob_oid;
+#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
+
static inline int is_empty_blob_sha1(const unsigned char *sha1)
{
return !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);
}
+static inline int is_empty_blob_oid(const struct object_id *oid)
+{
+ return !hashcmp(oid->hash, EMPTY_BLOB_SHA1_BIN);
+}
+
+static inline int is_empty_tree_sha1(const unsigned char *sha1)
+{
+ return !hashcmp(sha1, EMPTY_TREE_SHA1_BIN);
+}
+
+static inline int is_empty_tree_oid(const struct object_id *oid)
+{
+ return !hashcmp(oid->hash, EMPTY_TREE_SHA1_BIN);
+}
+
+
int git_mkstemp(char *path, size_t n, const char *template);
/* set default permissions by passing mode arguments to open(2) */
diff --git a/sha1_file.c b/sha1_file.c
index 1e23fc186a02..21cf923bcf1f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -38,6 +38,12 @@ static inline uintmax_t sz_fmt(size_t s) { return s; }
const unsigned char null_sha1[20];
const struct object_id null_oid;
+const struct object_id empty_tree_oid = {
+ EMPTY_TREE_SHA1_BIN_LITERAL
+};
+const struct object_id empty_blob_oid = {
+ EMPTY_BLOB_SHA1_BIN_LITERAL
+};
/*
* This is meant to hold a *small* number of objects that you would
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 2/8] diff.c: remove output_prefix_length field
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
2016-08-19 23:34 ` [PATCH v9 1/8] cache: add empty_tree_oid object and helper function Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 3/8] graph: add support for --line-prefix on all graph-aware output Jacob Keller
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt
From: Junio C Hamano <gitster@pobox.com>
"diff/log --stat" has a logic that determines the display columns
available for the diffstat part of the output and apportions it for
pathnames and diffstat graph automatically.
5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)
added the output_prefix_length field to diff_options structure to
allow this logic to subtract the display columns used for the
history graph part from the total "terminal width"; this matters
when the "git log --graph -p" option is in use.
The field must be set to the number of display columns needed to
show the output from the output_prefix() callback, which is error
prone. As there is only one user of the field, and the user has the
actual value of the prefix string, let's get rid of the field and
have the user count the display width itself.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 2 +-
diff.h | 1 -
graph.c | 2 --
3 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index 534c12e28ea8..50bef1f07658 100644
--- a/diff.c
+++ b/diff.c
@@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
*/
if (options->stat_width == -1)
- width = term_columns() - options->output_prefix_length;
+ width = term_columns() - strlen(line_prefix);
else
width = options->stat_width ? options->stat_width : 80;
number_width = decimal_width(max_change) > number_width ?
diff --git a/diff.h b/diff.h
index 7883729edf10..747a204d75a4 100644
--- a/diff.h
+++ b/diff.h
@@ -174,7 +174,6 @@ struct diff_options {
diff_format_fn_t format_callback;
void *format_callback_data;
diff_prefix_fn_t output_prefix;
- int output_prefix_length;
void *output_prefix_data;
int diff_path_counter;
diff --git a/graph.c b/graph.c
index dd1720148dc5..a46803840511 100644
--- a/graph.c
+++ b/graph.c
@@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void
assert(opt);
assert(graph);
- opt->output_prefix_length = graph->width;
strbuf_reset(&msgbuf);
graph_padding_line(graph, &msgbuf);
return &msgbuf;
@@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt)
*/
opt->diffopt.output_prefix = diff_output_prefix_callback;
opt->diffopt.output_prefix_data = graph;
- opt->diffopt.output_prefix_length = 0;
return graph;
}
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 3/8] graph: add support for --line-prefix on all graph-aware output
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
2016-08-19 23:34 ` [PATCH v9 1/8] cache: add empty_tree_oid object and helper function Jacob Keller
2016-08-19 23:34 ` [PATCH v9 2/8] diff.c: remove output_prefix_length field Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 4/8] diff: prepare for additional submodule formats Jacob Keller
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Add an extension to git-diff and git-log (and any other graph-aware
displayable output) such that "--line-prefix=<string>" will print the
additional line-prefix on every line of output.
To make this work, we have to fix a few bugs in the graph API that force
graph_show_commit_msg to be used only when you have a valid graph.
Additionally, we extend the default_diff_output_prefix handler to work
even when no graph is enabled.
This is somewhat of a hack on top of the graph API, but I think it
should be acceptable here.
This will be used by a future extension of submodule display which
displays the submodule diff as the actual diff between the pre and post
commit in the submodule project.
Add some tests for both git-log and git-diff to ensure that the prefix
is honored correctly.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/diff-options.txt | 3 +
builtin/rev-list.c | 70 ++---
diff.c | 7 +
diff.h | 2 +
graph.c | 98 ++++---
graph.h | 22 +-
log-tree.c | 5 +-
t/t4013-diff-various.sh | 6 +
...diff.diff_--line-prefix=abc_master_master^_side | 29 ++
t/t4013/diff.diff_--line-prefix_--cached_--_file0 | 15 +
t/t4202-log.sh | 323 +++++++++++++++++++++
11 files changed, 502 insertions(+), 78 deletions(-)
create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 705a87394200..cc4342e2034d 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -569,5 +569,8 @@ endif::git-format-patch[]
--no-prefix::
Do not show any source or destination prefix.
+--line-prefix=<prefix>::
+ Prepend an additional prefix to every line of output.
+
For more detailed explanation on these common options, see also
linkgit:gitdiffcore[7].
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0ba82b1635b6..8479f6ed28aa 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void *data)
ctx.fmt = revs->commit_format;
ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(&ctx, commit, &buf);
- if (revs->graph) {
- if (buf.len) {
- if (revs->commit_format != CMIT_FMT_ONELINE)
- graph_show_oneline(revs->graph);
+ if (buf.len) {
+ if (revs->commit_format != CMIT_FMT_ONELINE)
+ graph_show_oneline(revs->graph);
- graph_show_commit_msg(revs->graph, &buf);
+ graph_show_commit_msg(revs->graph, stdout, &buf);
- /*
- * Add a newline after the commit message.
- *
- * Usually, this newline produces a blank
- * padding line between entries, in which case
- * we need to add graph padding on this line.
- *
- * However, the commit message may not end in a
- * newline. In this case the newline simply
- * ends the last line of the commit message,
- * and we don't need any graph output. (This
- * always happens with CMIT_FMT_ONELINE, and it
- * happens with CMIT_FMT_USERFORMAT when the
- * format doesn't explicitly end in a newline.)
- */
- if (buf.len && buf.buf[buf.len - 1] == '\n')
- graph_show_padding(revs->graph);
- putchar('\n');
- } else {
- /*
- * If the message buffer is empty, just show
- * the rest of the graph output for this
- * commit.
- */
- if (graph_show_remainder(revs->graph))
- putchar('\n');
- if (revs->commit_format == CMIT_FMT_ONELINE)
- putchar('\n');
- }
+ /*
+ * Add a newline after the commit message.
+ *
+ * Usually, this newline produces a blank
+ * padding line between entries, in which case
+ * we need to add graph padding on this line.
+ *
+ * However, the commit message may not end in a
+ * newline. In this case the newline simply
+ * ends the last line of the commit message,
+ * and we don't need any graph output. (This
+ * always happens with CMIT_FMT_ONELINE, and it
+ * happens with CMIT_FMT_USERFORMAT when the
+ * format doesn't explicitly end in a newline.)
+ */
+ if (buf.len && buf.buf[buf.len - 1] == '\n')
+ graph_show_padding(revs->graph);
+ putchar('\n');
} else {
- if (revs->commit_format != CMIT_FMT_USERFORMAT ||
- buf.len) {
- fwrite(buf.buf, 1, buf.len, stdout);
- putchar(info->hdr_termination);
- }
+ /*
+ * If the message buffer is empty, just show
+ * the rest of the graph output for this
+ * commit.
+ */
+ if (graph_show_remainder(revs->graph))
+ putchar('\n');
+ if (revs->commit_format == CMIT_FMT_ONELINE)
+ putchar('\n');
}
strbuf_release(&buf);
} else {
diff --git a/diff.c b/diff.c
index 50bef1f07658..e57cf39ad109 100644
--- a/diff.c
+++ b/diff.c
@@ -18,6 +18,7 @@
#include "ll-merge.h"
#include "string-list.h"
#include "argv-array.h"
+#include "graph.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -3966,6 +3967,12 @@ int diff_opt_parse(struct diff_options *options,
options->a_prefix = optarg;
return argcount;
}
+ else if ((argcount = parse_long_opt("line-prefix", av, &optarg))) {
+ options->line_prefix = optarg;
+ options->line_prefix_length = strlen(options->line_prefix);
+ graph_setup_line_prefix(options);
+ return argcount;
+ }
else if ((argcount = parse_long_opt("dst-prefix", av, &optarg))) {
options->b_prefix = optarg;
return argcount;
diff --git a/diff.h b/diff.h
index 747a204d75a4..1f57aad25c71 100644
--- a/diff.h
+++ b/diff.h
@@ -115,6 +115,8 @@ struct diff_options {
const char *pickaxe;
const char *single_follow;
const char *a_prefix, *b_prefix;
+ const char *line_prefix;
+ size_t line_prefix_length;
unsigned flags;
unsigned touched_flags;
diff --git a/graph.c b/graph.c
index a46803840511..06f1139f2e20 100644
--- a/graph.c
+++ b/graph.c
@@ -2,7 +2,6 @@
#include "commit.h"
#include "color.h"
#include "graph.h"
-#include "diff.h"
#include "revision.h"
/* Internal API */
@@ -28,8 +27,15 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
* responsible for printing this line's graph (perhaps via
* graph_show_commit() or graph_show_oneline()) before calling
* graph_show_strbuf().
+ *
+ * Note that unlike some other graph display functions, you must pass the file
+ * handle directly. It is assumed that this is the same file handle as the
+ * file specified by the graph diff options. This is necessary so that
+ * graph_show_strbuf can be called even with a NULL graph.
*/
-static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb);
+static void graph_show_strbuf(struct git_graph *graph,
+ FILE *file,
+ struct strbuf const *sb);
/*
* TODO:
@@ -59,6 +65,17 @@ enum graph_state {
GRAPH_COLLAPSING
};
+static void graph_show_line_prefix(const struct diff_options *diffopt)
+{
+ if (!diffopt || !diffopt->line_prefix)
+ return;
+
+ fwrite(diffopt->line_prefix,
+ sizeof(char),
+ diffopt->line_prefix_length,
+ diffopt->file);
+}
+
static const char **column_colors;
static unsigned short column_colors_max;
@@ -195,13 +212,28 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void
static struct strbuf msgbuf = STRBUF_INIT;
assert(opt);
- assert(graph);
strbuf_reset(&msgbuf);
- graph_padding_line(graph, &msgbuf);
+ if (opt->line_prefix)
+ strbuf_add(&msgbuf, opt->line_prefix,
+ opt->line_prefix_length);
+ if (graph)
+ graph_padding_line(graph, &msgbuf);
return &msgbuf;
}
+static const struct diff_options *default_diffopt;
+
+void graph_setup_line_prefix(struct diff_options *diffopt)
+{
+ default_diffopt = diffopt;
+
+ /* setup an output prefix callback if necessary */
+ if (diffopt && !diffopt->output_prefix)
+ diffopt->output_prefix = diff_output_prefix_callback;
+}
+
+
struct git_graph *graph_init(struct rev_info *opt)
{
struct git_graph *graph = xmalloc(sizeof(struct git_graph));
@@ -1183,6 +1215,8 @@ void graph_show_commit(struct git_graph *graph)
struct strbuf msgbuf = STRBUF_INIT;
int shown_commit_line = 0;
+ graph_show_line_prefix(default_diffopt);
+
if (!graph)
return;
@@ -1200,8 +1234,10 @@ void graph_show_commit(struct git_graph *graph)
shown_commit_line = graph_next_line(graph, &msgbuf);
fwrite(msgbuf.buf, sizeof(char), msgbuf.len,
graph->revs->diffopt.file);
- if (!shown_commit_line)
+ if (!shown_commit_line) {
putc('\n', graph->revs->diffopt.file);
+ graph_show_line_prefix(&graph->revs->diffopt);
+ }
strbuf_setlen(&msgbuf, 0);
}
@@ -1212,6 +1248,8 @@ void graph_show_oneline(struct git_graph *graph)
{
struct strbuf msgbuf = STRBUF_INIT;
+ graph_show_line_prefix(default_diffopt);
+
if (!graph)
return;
@@ -1224,6 +1262,8 @@ void graph_show_padding(struct git_graph *graph)
{
struct strbuf msgbuf = STRBUF_INIT;
+ graph_show_line_prefix(default_diffopt);
+
if (!graph)
return;
@@ -1237,6 +1277,8 @@ int graph_show_remainder(struct git_graph *graph)
struct strbuf msgbuf = STRBUF_INIT;
int shown = 0;
+ graph_show_line_prefix(default_diffopt);
+
if (!graph)
return 0;
@@ -1250,27 +1292,24 @@ int graph_show_remainder(struct git_graph *graph)
strbuf_setlen(&msgbuf, 0);
shown = 1;
- if (!graph_is_commit_finished(graph))
+ if (!graph_is_commit_finished(graph)) {
putc('\n', graph->revs->diffopt.file);
- else
+ graph_show_line_prefix(&graph->revs->diffopt);
+ } else {
break;
+ }
}
strbuf_release(&msgbuf);
return shown;
}
-
-static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb)
+static void graph_show_strbuf(struct git_graph *graph,
+ FILE *file,
+ struct strbuf const *sb)
{
char *p;
- if (!graph) {
- fwrite(sb->buf, sizeof(char), sb->len,
- graph->revs->diffopt.file);
- return;
- }
-
/*
* Print the strbuf line by line,
* and display the graph info before each line but the first.
@@ -1285,7 +1324,7 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb)
} else {
len = (sb->buf + sb->len) - p;
}
- fwrite(p, sizeof(char), len, graph->revs->diffopt.file);
+ fwrite(p, sizeof(char), len, file);
if (next_p && *next_p != '\0')
graph_show_oneline(graph);
p = next_p;
@@ -1293,29 +1332,20 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb)
}
void graph_show_commit_msg(struct git_graph *graph,
+ FILE *file,
struct strbuf const *sb)
{
int newline_terminated;
- if (!graph) {
- /*
- * If there's no graph, just print the message buffer.
- *
- * The message buffer for CMIT_FMT_ONELINE and
- * CMIT_FMT_USERFORMAT are already missing a terminating
- * newline. All of the other formats should have it.
- */
- fwrite(sb->buf, sizeof(char), sb->len,
- graph->revs->diffopt.file);
- return;
- }
-
- newline_terminated = (sb->len && sb->buf[sb->len - 1] == '\n');
-
/*
* Show the commit message
*/
- graph_show_strbuf(graph, sb);
+ graph_show_strbuf(graph, file, sb);
+
+ if (!graph)
+ return;
+
+ newline_terminated = (sb->len && sb->buf[sb->len - 1] == '\n');
/*
* If there is more output needed for this commit, show it now
@@ -1327,7 +1357,7 @@ void graph_show_commit_msg(struct git_graph *graph,
* new line.
*/
if (!newline_terminated)
- putc('\n', graph->revs->diffopt.file);
+ putc('\n', file);
graph_show_remainder(graph);
@@ -1335,6 +1365,6 @@ void graph_show_commit_msg(struct git_graph *graph,
* If sb ends with a newline, our output should too.
*/
if (newline_terminated)
- putc('\n', graph->revs->diffopt.file);
+ putc('\n', file);
}
}
diff --git a/graph.h b/graph.h
index 3f48c19b6208..af623390b605 100644
--- a/graph.h
+++ b/graph.h
@@ -1,9 +1,22 @@
#ifndef GRAPH_H
#define GRAPH_H
+#include "diff.h"
/* A graph is a pointer to this opaque structure */
struct git_graph;
+/*
+ * Called to setup global display of line_prefix diff option.
+ *
+ * Passed a diff_options structure which indicates the line_prefix and the
+ * file to output the prefix to. This is sort of a hack used so that the
+ * line_prefix will be honored by all flows which also honor "--graph"
+ * regardless of whether a graph has actually been setup. The normal graph
+ * flow will honor the exact diff_options passed, but a NULL graph will cause
+ * display of a line_prefix to stdout.
+ */
+void graph_setup_line_prefix(struct diff_options *diffopt);
+
/*
* Set up a custom scheme for column colors.
*
@@ -113,7 +126,14 @@ int graph_show_remainder(struct git_graph *graph);
* missing a terminating newline (including if it is empty), the output
* printed by graph_show_commit_msg() will also be missing a terminating
* newline.
+ *
+ * Note that unlike some other graph display functions, you must pass the file
+ * handle directly. It is assumed that this is the same file handle as the
+ * file specified by the graph diff options. This is necessary so that
+ * graph_show_commit_msg can be called even with a NULL graph.
*/
-void graph_show_commit_msg(struct git_graph *graph, struct strbuf const *sb);
+void graph_show_commit_msg(struct git_graph *graph,
+ FILE *file,
+ struct strbuf const *sb);
#endif /* GRAPH_H */
diff --git a/log-tree.c b/log-tree.c
index bfb735c84556..8c2415747a26 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -715,10 +715,7 @@ void show_log(struct rev_info *opt)
else
opt->missing_newline = 0;
- if (opt->graph)
- graph_show_commit_msg(opt->graph, &msgbuf);
- else
- fwrite(msgbuf.buf, sizeof(char), msgbuf.len, opt->diffopt.file);
+ graph_show_commit_msg(opt->graph, opt->diffopt.file, &msgbuf);
if (opt->use_terminator && !commit_format_is_empty(opt->commit_format)) {
if (!opt->missing_newline)
graph_show_padding(opt->graph);
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 94ef5000e787..566817e2efdc 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -306,6 +306,8 @@ diff --no-index --name-status dir2 dir
diff --no-index --name-status -- dir2 dir
diff --no-index dir dir3
diff master master^ side
+# Can't use spaces...
+diff --line-prefix=abc master master^ side
diff --dirstat master~1 master~2
diff --dirstat initial rearrange
diff --dirstat-by-file initial rearrange
@@ -325,6 +327,10 @@ test_expect_success 'diff --cached -- file on unborn branch' '
git diff --cached -- file0 >result &&
test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result
'
+test_expect_success 'diff --line-prefix with spaces' '
+ git diff --line-prefix="| | | " --cached -- file0 >result &&
+ test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--line-prefix_--cached_--_file0" result
+'
test_expect_success 'diff-tree --stdin with log formatting' '
cat >expect <<-\EOF &&
diff --git a/t/t4013/diff.diff_--line-prefix=abc_master_master^_side b/t/t4013/diff.diff_--line-prefix=abc_master_master^_side
new file mode 100644
index 000000000000..99f91e7f0e32
--- /dev/null
+++ b/t/t4013/diff.diff_--line-prefix=abc_master_master^_side
@@ -0,0 +1,29 @@
+$ git diff --line-prefix=abc master master^ side
+abcdiff --cc dir/sub
+abcindex cead32e,7289e35..992913c
+abc--- a/dir/sub
+abc+++ b/dir/sub
+abc@@@ -1,6 -1,4 +1,8 @@@
+abc A
+abc B
+abc +C
+abc +D
+abc +E
+abc +F
+abc+ 1
+abc+ 2
+abcdiff --cc file0
+abcindex b414108,f4615da..10a8a9f
+abc--- a/file0
+abc+++ b/file0
+abc@@@ -1,6 -1,6 +1,9 @@@
+abc 1
+abc 2
+abc 3
+abc +4
+abc +5
+abc +6
+abc+ A
+abc+ B
+abc+ C
+$
diff --git a/t/t4013/diff.diff_--line-prefix_--cached_--_file0 b/t/t4013/diff.diff_--line-prefix_--cached_--_file0
new file mode 100644
index 000000000000..f41ba4d36aa1
--- /dev/null
+++ b/t/t4013/diff.diff_--line-prefix_--cached_--_file0
@@ -0,0 +1,15 @@
+| | | diff --git a/file0 b/file0
+| | | new file mode 100644
+| | | index 0000000..10a8a9f
+| | | --- /dev/null
+| | | +++ b/file0
+| | | @@ -0,0 +1,9 @@
+| | | +1
+| | | +2
+| | | +3
+| | | +4
+| | | +5
+| | | +6
+| | | +A
+| | | +B
+| | | +C
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c36e09..1ccbd5948a73 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -187,6 +187,16 @@ test_expect_success 'git log --no-walk=sorted <commits> sorts by commit time' '
test_cmp expect actual
'
+cat > expect << EOF
+=== 804a787 sixth
+=== 394ef78 fifth
+=== 5d31159 fourth
+EOF
+test_expect_success 'git log --line-prefix="=== " --no-walk <commits> sorts by commit time' '
+ git log --line-prefix="=== " --no-walk --oneline 5d31159 804a787 394ef78 > actual &&
+ test_cmp expect actual
+'
+
cat > expect << EOF
5d31159 fourth
804a787 sixth
@@ -284,6 +294,21 @@ test_expect_success 'simple log --graph' '
test_cmp expect actual
'
+cat > expect <<EOF
+123 * Second
+123 * sixth
+123 * fifth
+123 * fourth
+123 * third
+123 * second
+123 * initial
+EOF
+
+test_expect_success 'simple log --graph --line-prefix="123 "' '
+ git log --graph --line-prefix="123 " --pretty=tformat:%s >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'set up merge history' '
git checkout -b side HEAD~4 &&
test_commit side-1 1 1 &&
@@ -313,6 +338,27 @@ test_expect_success 'log --graph with merge' '
test_cmp expect actual
'
+cat > expect <<\EOF
+| | | * Merge branch 'side'
+| | | |\
+| | | | * side-2
+| | | | * side-1
+| | | * | Second
+| | | * | sixth
+| | | * | fifth
+| | | * | fourth
+| | | |/
+| | | * third
+| | | * second
+| | | * initial
+EOF
+
+test_expect_success 'log --graph --line-prefix="| | | " with merge' '
+ git log --line-prefix="| | | " --graph --date-order --pretty=tformat:%s |
+ sed "s/ *\$//" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'log --raw --graph -m with merge' '
git log --raw --graph --oneline -m master | head -n 500 >actual &&
grep "initial" actual
@@ -867,6 +913,283 @@ test_expect_success 'log --graph with diff and stats' '
test_i18ncmp expect actual.sanitized
'
+cat >expect <<\EOF
+*** * commit COMMIT_OBJECT_NAME
+*** |\ Merge: MERGE_PARENTS
+*** | | Author: A U Thor <author@example.com>
+*** | |
+*** | | Merge HEADS DESCRIPTION
+*** | |
+*** | * commit COMMIT_OBJECT_NAME
+*** | | Author: A U Thor <author@example.com>
+*** | |
+*** | | reach
+*** | | ---
+*** | | reach.t | 1 +
+*** | | 1 file changed, 1 insertion(+)
+*** | |
+*** | | diff --git a/reach.t b/reach.t
+*** | | new file mode 100644
+*** | | index 0000000..10c9591
+*** | | --- /dev/null
+*** | | +++ b/reach.t
+*** | | @@ -0,0 +1 @@
+*** | | +reach
+*** | |
+*** | \
+*** *-. \ commit COMMIT_OBJECT_NAME
+*** |\ \ \ Merge: MERGE_PARENTS
+*** | | | | Author: A U Thor <author@example.com>
+*** | | | |
+*** | | | | Merge HEADS DESCRIPTION
+*** | | | |
+*** | | * | commit COMMIT_OBJECT_NAME
+*** | | |/ Author: A U Thor <author@example.com>
+*** | | |
+*** | | | octopus-b
+*** | | | ---
+*** | | | octopus-b.t | 1 +
+*** | | | 1 file changed, 1 insertion(+)
+*** | | |
+*** | | | diff --git a/octopus-b.t b/octopus-b.t
+*** | | | new file mode 100644
+*** | | | index 0000000..d5fcad0
+*** | | | --- /dev/null
+*** | | | +++ b/octopus-b.t
+*** | | | @@ -0,0 +1 @@
+*** | | | +octopus-b
+*** | | |
+*** | * | commit COMMIT_OBJECT_NAME
+*** | |/ Author: A U Thor <author@example.com>
+*** | |
+*** | | octopus-a
+*** | | ---
+*** | | octopus-a.t | 1 +
+*** | | 1 file changed, 1 insertion(+)
+*** | |
+*** | | diff --git a/octopus-a.t b/octopus-a.t
+*** | | new file mode 100644
+*** | | index 0000000..11ee015
+*** | | --- /dev/null
+*** | | +++ b/octopus-a.t
+*** | | @@ -0,0 +1 @@
+*** | | +octopus-a
+*** | |
+*** * | commit COMMIT_OBJECT_NAME
+*** |/ Author: A U Thor <author@example.com>
+*** |
+*** | seventh
+*** | ---
+*** | seventh.t | 1 +
+*** | 1 file changed, 1 insertion(+)
+*** |
+*** | diff --git a/seventh.t b/seventh.t
+*** | new file mode 100644
+*** | index 0000000..9744ffc
+*** | --- /dev/null
+*** | +++ b/seventh.t
+*** | @@ -0,0 +1 @@
+*** | +seventh
+*** |
+*** * commit COMMIT_OBJECT_NAME
+*** |\ Merge: MERGE_PARENTS
+*** | | Author: A U Thor <author@example.com>
+*** | |
+*** | | Merge branch 'tangle'
+*** | |
+*** | * commit COMMIT_OBJECT_NAME
+*** | |\ Merge: MERGE_PARENTS
+*** | | | Author: A U Thor <author@example.com>
+*** | | |
+*** | | | Merge branch 'side' (early part) into tangle
+*** | | |
+*** | * | commit COMMIT_OBJECT_NAME
+*** | |\ \ Merge: MERGE_PARENTS
+*** | | | | Author: A U Thor <author@example.com>
+*** | | | |
+*** | | | | Merge branch 'master' (early part) into tangle
+*** | | | |
+*** | * | | commit COMMIT_OBJECT_NAME
+*** | | | | Author: A U Thor <author@example.com>
+*** | | | |
+*** | | | | tangle-a
+*** | | | | ---
+*** | | | | tangle-a | 1 +
+*** | | | | 1 file changed, 1 insertion(+)
+*** | | | |
+*** | | | | diff --git a/tangle-a b/tangle-a
+*** | | | | new file mode 100644
+*** | | | | index 0000000..7898192
+*** | | | | --- /dev/null
+*** | | | | +++ b/tangle-a
+*** | | | | @@ -0,0 +1 @@
+*** | | | | +a
+*** | | | |
+*** * | | | commit COMMIT_OBJECT_NAME
+*** |\ \ \ \ Merge: MERGE_PARENTS
+*** | | | | | Author: A U Thor <author@example.com>
+*** | | | | |
+*** | | | | | Merge branch 'side'
+*** | | | | |
+*** | * | | | commit COMMIT_OBJECT_NAME
+*** | | |_|/ Author: A U Thor <author@example.com>
+*** | |/| |
+*** | | | | side-2
+*** | | | | ---
+*** | | | | 2 | 1 +
+*** | | | | 1 file changed, 1 insertion(+)
+*** | | | |
+*** | | | | diff --git a/2 b/2
+*** | | | | new file mode 100644
+*** | | | | index 0000000..0cfbf08
+*** | | | | --- /dev/null
+*** | | | | +++ b/2
+*** | | | | @@ -0,0 +1 @@
+*** | | | | +2
+*** | | | |
+*** | * | | commit COMMIT_OBJECT_NAME
+*** | | | | Author: A U Thor <author@example.com>
+*** | | | |
+*** | | | | side-1
+*** | | | | ---
+*** | | | | 1 | 1 +
+*** | | | | 1 file changed, 1 insertion(+)
+*** | | | |
+*** | | | | diff --git a/1 b/1
+*** | | | | new file mode 100644
+*** | | | | index 0000000..d00491f
+*** | | | | --- /dev/null
+*** | | | | +++ b/1
+*** | | | | @@ -0,0 +1 @@
+*** | | | | +1
+*** | | | |
+*** * | | | commit COMMIT_OBJECT_NAME
+*** | | | | Author: A U Thor <author@example.com>
+*** | | | |
+*** | | | | Second
+*** | | | | ---
+*** | | | | one | 1 +
+*** | | | | 1 file changed, 1 insertion(+)
+*** | | | |
+*** | | | | diff --git a/one b/one
+*** | | | | new file mode 100644
+*** | | | | index 0000000..9a33383
+*** | | | | --- /dev/null
+*** | | | | +++ b/one
+*** | | | | @@ -0,0 +1 @@
+*** | | | | +case
+*** | | | |
+*** * | | | commit COMMIT_OBJECT_NAME
+*** | |_|/ Author: A U Thor <author@example.com>
+*** |/| |
+*** | | | sixth
+*** | | | ---
+*** | | | a/two | 1 -
+*** | | | 1 file changed, 1 deletion(-)
+*** | | |
+*** | | | diff --git a/a/two b/a/two
+*** | | | deleted file mode 100644
+*** | | | index 9245af5..0000000
+*** | | | --- a/a/two
+*** | | | +++ /dev/null
+*** | | | @@ -1 +0,0 @@
+*** | | | -ni
+*** | | |
+*** * | | commit COMMIT_OBJECT_NAME
+*** | | | Author: A U Thor <author@example.com>
+*** | | |
+*** | | | fifth
+*** | | | ---
+*** | | | a/two | 1 +
+*** | | | 1 file changed, 1 insertion(+)
+*** | | |
+*** | | | diff --git a/a/two b/a/two
+*** | | | new file mode 100644
+*** | | | index 0000000..9245af5
+*** | | | --- /dev/null
+*** | | | +++ b/a/two
+*** | | | @@ -0,0 +1 @@
+*** | | | +ni
+*** | | |
+*** * | | commit COMMIT_OBJECT_NAME
+*** |/ / Author: A U Thor <author@example.com>
+*** | |
+*** | | fourth
+*** | | ---
+*** | | ein | 1 +
+*** | | 1 file changed, 1 insertion(+)
+*** | |
+*** | | diff --git a/ein b/ein
+*** | | new file mode 100644
+*** | | index 0000000..9d7e69f
+*** | | --- /dev/null
+*** | | +++ b/ein
+*** | | @@ -0,0 +1 @@
+*** | | +ichi
+*** | |
+*** * | commit COMMIT_OBJECT_NAME
+*** |/ Author: A U Thor <author@example.com>
+*** |
+*** | third
+*** | ---
+*** | ichi | 1 +
+*** | one | 1 -
+*** | 2 files changed, 1 insertion(+), 1 deletion(-)
+*** |
+*** | diff --git a/ichi b/ichi
+*** | new file mode 100644
+*** | index 0000000..9d7e69f
+*** | --- /dev/null
+*** | +++ b/ichi
+*** | @@ -0,0 +1 @@
+*** | +ichi
+*** | diff --git a/one b/one
+*** | deleted file mode 100644
+*** | index 9d7e69f..0000000
+*** | --- a/one
+*** | +++ /dev/null
+*** | @@ -1 +0,0 @@
+*** | -ichi
+*** |
+*** * commit COMMIT_OBJECT_NAME
+*** | Author: A U Thor <author@example.com>
+*** |
+*** | second
+*** | ---
+*** | one | 2 +-
+*** | 1 file changed, 1 insertion(+), 1 deletion(-)
+*** |
+*** | diff --git a/one b/one
+*** | index 5626abf..9d7e69f 100644
+*** | --- a/one
+*** | +++ b/one
+*** | @@ -1 +1 @@
+*** | -one
+*** | +ichi
+*** |
+*** * commit COMMIT_OBJECT_NAME
+*** Author: A U Thor <author@example.com>
+***
+*** initial
+*** ---
+*** one | 1 +
+*** 1 file changed, 1 insertion(+)
+***
+*** diff --git a/one b/one
+*** new file mode 100644
+*** index 0000000..5626abf
+*** --- /dev/null
+*** +++ b/one
+*** @@ -0,0 +1 @@
+*** +one
+EOF
+
+test_expect_success 'log --line-prefix="*** " --graph with diff and stats' '
+ git log --line-prefix="*** " --no-renames --graph --pretty=short --stat -p >actual &&
+ sanitize_output >actual.sanitized <actual &&
+ test_i18ncmp expect actual.sanitized
+'
+
test_expect_success 'dotdot is a parent directory' '
mkdir -p a/b &&
( echo sixth && echo fifth ) >expect &&
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 4/8] diff: prepare for additional submodule formats
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
` (2 preceding siblings ...)
2016-08-19 23:34 ` [PATCH v9 3/8] graph: add support for --line-prefix on all graph-aware output Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 5/8] submodule: allow add_submodule_odb to work even if path is not checked out Jacob Keller
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
A future patch will add a new format for displaying the difference of
a submodule. Make it easier by changing how we store the current
selected format. Replace the DIFF_OPT flag with an enumeration, as each
format will be mutually exclusive.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
diff.c | 12 ++++++------
diff.h | 7 ++++++-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index e57cf39ad109..d6b321da3d1d 100644
--- a/diff.c
+++ b/diff.c
@@ -132,9 +132,9 @@ static int parse_dirstat_params(struct diff_options *options, const char *params
static int parse_submodule_params(struct diff_options *options, const char *value)
{
if (!strcmp(value, "log"))
- DIFF_OPT_SET(options, SUBMODULE_LOG);
+ options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
- DIFF_OPT_CLR(options, SUBMODULE_LOG);
+ options->submodule_format = DIFF_SUBMODULE_SHORT;
else
return -1;
return 0;
@@ -2300,9 +2300,9 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
- if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
- (!one->mode || S_ISGITLINK(one->mode)) &&
- (!two->mode || S_ISGITLINK(two->mode))) {
+ if (o->submodule_format == DIFF_SUBMODULE_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->path ? one->path : two->path,
@@ -3916,7 +3916,7 @@ int diff_opt_parse(struct diff_options *options,
DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
handle_ignore_submodules_arg(options, arg);
} else if (!strcmp(arg, "--submodule"))
- DIFF_OPT_SET(options, SUBMODULE_LOG);
+ options->submodule_format = DIFF_SUBMODULE_LOG;
else if (skip_prefix(arg, "--submodule=", &arg))
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
diff --git a/diff.h b/diff.h
index 1f57aad25c71..43b353aea091 100644
--- a/diff.h
+++ b/diff.h
@@ -83,7 +83,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#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_LOG (1 << 23)
#define DIFF_OPT_DIRTY_SUBMODULES (1 << 24)
#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)
@@ -110,6 +109,11 @@ enum diff_words_type {
DIFF_WORDS_COLOR
};
+enum diff_submodule_format {
+ DIFF_SUBMODULE_SHORT = 0,
+ DIFF_SUBMODULE_LOG
+};
+
struct diff_options {
const char *orderfile;
const char *pickaxe;
@@ -157,6 +161,7 @@ struct diff_options {
int stat_count;
const char *word_regex;
enum diff_words_type word_diff;
+ enum diff_submodule_format submodule_format;
/* this is set by diffcore for DIFF_FORMAT_PATCH */
int found_changes;
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 5/8] submodule: allow add_submodule_odb to work even if path is not checked out
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
` (3 preceding siblings ...)
2016-08-19 23:34 ` [PATCH v9 4/8] diff: prepare for additional submodule formats Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 6/8] submodule: convert show_submodule_summary to use struct object_id * Jacob Keller
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Currently, do_submodule_path will first try to locate the git directory
using read_gitfile on <path to submodule>/.git. If this fails, it goes
ahead and assumes the path is actually the git directory. This is good
as it allows submodules which aren't stored in the superproject's .git
directory to function correctly. However, in some cases the submodule is
no longer locally checked out, but still has object data stored in the
parent project's .git/modules/<path to submodule>.
To make this work, add code to check if we found a valid git directory.
If we haven't, then try the standard location of module data instead.
This has the advantage of allowing callers of do_submodule_path
(add_submodule_odb) to correctly function even if the submodule isn't
currently checked out, but was previously initialized.
Update the wording of the submodule log diff format to correctly
display that the submodule is "not initialized" instead of "not checked
out"
Add tests to ensure that even once we remove the submodule directory, it
works by checking in the .git directory.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
path.c | 16 ++++
submodule.c | 2 +-
t/t4059-diff-submodule-not-initialized.sh | 127 ++++++++++++++++++++++++++++++
3 files changed, 144 insertions(+), 1 deletion(-)
create mode 100755 t/t4059-diff-submodule-not-initialized.sh
diff --git a/path.c b/path.c
index fe3c4d96c6d8..081a22c1163c 100644
--- a/path.c
+++ b/path.c
@@ -6,6 +6,7 @@
#include "string-list.h"
#include "dir.h"
#include "worktree.h"
+#include "submodule-config.h"
static int get_st_mode_bits(const char *path, int *mode)
{
@@ -474,6 +475,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
const char *git_dir;
struct strbuf git_submodule_common_dir = STRBUF_INIT;
struct strbuf git_submodule_dir = STRBUF_INIT;
+ const struct submodule *submodule_config;
strbuf_addstr(buf, path);
strbuf_complete(buf, '/');
@@ -484,6 +486,20 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
strbuf_reset(buf);
strbuf_addstr(buf, git_dir);
}
+ if (!is_git_directory(buf->buf)) {
+ strbuf_reset(buf);
+ /*
+ * Lookup the submodule name from the config. If that fails
+ * fall back to assuming the path is the name.
+ */
+ submodule_config = submodule_from_path(null_sha1, path);
+ if (submodule_config)
+ strbuf_git_path(buf, "%s/%s", "modules",
+ submodule_config->name);
+ else
+ strbuf_git_path(buf, "%s/%s", "modules", path);
+ }
+
strbuf_addch(buf, '/');
strbuf_addbuf(&git_submodule_dir, buf);
diff --git a/submodule.c b/submodule.c
index 1b5cdfb7e784..e1a51b7506ff 100644
--- a/submodule.c
+++ b/submodule.c
@@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path,
if (is_null_sha1(two))
message = "(submodule deleted)";
else if (add_submodule_odb(path))
- message = "(not checked out)";
+ message = "(not initialized)";
else if (is_null_sha1(one))
message = "(new submodule)";
else if (!(left = lookup_commit_reference(one)) ||
diff --git a/t/t4059-diff-submodule-not-initialized.sh b/t/t4059-diff-submodule-not-initialized.sh
new file mode 100755
index 000000000000..c8775854d3c2
--- /dev/null
+++ b/t/t4059-diff-submodule-not-initialized.sh
@@ -0,0 +1,127 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Jacob Keller, based on t4041 by Jens Lehmann
+#
+
+test_description='Test for submodule diff on non-checked out submodule
+
+This test tries to verify that add_submodule_odb works when the submodule was
+initialized previously but the checkout has since been removed.
+'
+
+. ./test-lib.sh
+
+# Tested non-UTF-8 encoding
+test_encoding="ISO8859-1"
+
+# String "added" in German (translated with Google Translate), encoded in UTF-8,
+# used in sample commit log messages in add_file() function below.
+added=$(printf "hinzugef\303\274gt")
+
+add_file () {
+ (
+ cd "$1" &&
+ shift &&
+ for name
+ do
+ echo "$name" >"$name" &&
+ git add "$name" &&
+ test_tick &&
+ # "git commit -m" would break MinGW, as Windows refuse to pass
+ # $test_encoding encoded parameter to git.
+ echo "Add $name ($added $name)" | iconv -f utf-8 -t $test_encoding |
+ git -c "i18n.commitEncoding=$test_encoding" commit -F -
+ done >/dev/null &&
+ git rev-parse --short --verify HEAD
+ )
+}
+
+commit_file () {
+ test_tick &&
+ git commit "$@" -m "Commit $*" >/dev/null
+}
+
+test_expect_success 'setup - submodules' '
+ test_create_repo sm2 &&
+ add_file . foo &&
+ add_file sm2 foo1 foo2 &&
+ smhead1=$(git -C sm2 rev-parse --short --verify HEAD)
+'
+
+test_expect_success 'setup - git submodule add' '
+ git submodule add ./sm2 sm1 &&
+ commit_file sm1 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 0000000...$smhead1 (new submodule)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule directory removed' '
+ rm -rf sm1 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 0000000...$smhead1 (new submodule)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'setup - submodule multiple commits' '
+ git submodule update --checkout sm1 &&
+ smhead2=$(add_file sm1 foo3 foo4) &&
+ commit_file sm1 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $smhead1..$smhead2:
+ > Add foo4 ($added foo4)
+ > Add foo3 ($added foo3)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule removed multiple commits' '
+ rm -rf sm1 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $smhead1..$smhead2:
+ > Add foo4 ($added foo4)
+ > Add foo3 ($added foo3)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule not initialized in new clone' '
+ git clone . sm3 &&
+ git -C sm3 diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $smhead1...$smhead2 (not initialized)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'setup submodule moved' '
+ git submodule update --checkout sm1 &&
+ git mv sm1 sm4 &&
+ commit_file sm4 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm4 0000000...$smhead2 (new submodule)
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule moved then removed' '
+ smhead3=$(add_file sm4 foo6 foo7) &&
+ commit_file sm4 &&
+ rm -rf sm4 &&
+ git diff-tree -p --no-commit-id --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm4 $smhead2..$smhead3:
+ > Add foo7 ($added foo7)
+ > Add foo6 ($added foo6)
+ EOF
+ test_cmp expected actual
+'
+
+test_done
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 6/8] submodule: convert show_submodule_summary to use struct object_id *
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
` (4 preceding siblings ...)
2016-08-19 23:34 ` [PATCH v9 5/8] submodule: allow add_submodule_odb to work even if path is not checked out Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 7/8] submodule: refactor show_submodule_summary with helper function Jacob Keller
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Since we're going to be changing this function in a future patch, lets
go ahead and convert this to use object_id now.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
diff.c | 2 +-
submodule.c | 16 ++++++++--------
submodule.h | 2 +-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/diff.c b/diff.c
index d6b321da3d1d..16253b191f53 100644
--- a/diff.c
+++ b/diff.c
@@ -2307,7 +2307,7 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path : two->path,
line_prefix,
- one->oid.hash, two->oid.hash,
+ &one->oid, &two->oid,
two->dirty_submodule,
meta, del, add, reset);
return;
diff --git a/submodule.c b/submodule.c
index e1a51b7506ff..422353ccf6cc 100644
--- a/submodule.c
+++ b/submodule.c
@@ -335,7 +335,7 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
- unsigned char one[20], unsigned char two[20],
+ struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset)
{
@@ -345,14 +345,14 @@ void show_submodule_summary(FILE *f, const char *path,
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
- if (is_null_sha1(two))
+ if (is_null_oid(two))
message = "(submodule deleted)";
else if (add_submodule_odb(path))
message = "(not initialized)";
- else if (is_null_sha1(one))
+ else if (is_null_oid(one))
message = "(new submodule)";
- else if (!(left = lookup_commit_reference(one)) ||
- !(right = lookup_commit_reference(two)))
+ else if (!(left = lookup_commit_reference(one->hash)) ||
+ !(right = lookup_commit_reference(two->hash)))
message = "(commits not present)";
else if (prepare_submodule_summary(&rev, path, left, right,
&fast_forward, &fast_backward))
@@ -365,16 +365,16 @@ void show_submodule_summary(FILE *f, const char *path,
fprintf(f, "%sSubmodule %s contains modified content\n",
line_prefix, path);
- if (!hashcmp(one, two)) {
+ if (!oidcmp(one, two)) {
strbuf_release(&sb);
return;
}
strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
- find_unique_abbrev(one, DEFAULT_ABBREV));
+ find_unique_abbrev(one->hash, DEFAULT_ABBREV));
if (!fast_backward && !fast_forward)
strbuf_addch(&sb, '.');
- strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
+ strbuf_addf(&sb, "%s", find_unique_abbrev(two->hash, DEFAULT_ABBREV));
if (message)
strbuf_addf(&sb, " %s%s\n", message, reset);
else
diff --git a/submodule.h b/submodule.h
index 2af939099819..d83df57e24ff 100644
--- a/submodule.h
+++ b/submodule.h
@@ -43,7 +43,7 @@ const char *submodule_strategy_to_string(const struct submodule_update_strategy
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
void show_submodule_summary(FILE *f, const char *path,
const char *line_prefix,
- unsigned char one[20], unsigned char two[20],
+ struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
void set_config_fetch_recurse_submodules(int value);
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 7/8] submodule: refactor show_submodule_summary with helper function
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
` (5 preceding siblings ...)
2016-08-19 23:34 ` [PATCH v9 6/8] submodule: convert show_submodule_summary to use struct object_id * Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-19 23:34 ` [PATCH v9 8/8] diff: teach diff to display submodule difference with an inline diff Jacob Keller
2016-08-20 0:02 ` [PATCH v9 0/8] submodule show " Stefan Beller
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
A future patch is going to add a new submodule diff format which
displays an inline diff of the submodule changes. To make this easier,
and to ensure that both submodule diff formats use the same initial
header, factor out show_submodule_header() function which will print the
current submodule header line, and then leave the show_submodule_summary
function to lookup and print the submodule log format.
This does create one format change in that "(revision walker failed)"
will now be displayed on its own line rather than as part of the message
because we no longer perform this step directly in the header display
flow. However, this is a rare case as most causes of the failure will be
due to a missing commit which we already check for and avoid previously.
flow. However, this is a rare case and shouldn't impact much.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
submodule.c | 115 +++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 82 insertions(+), 33 deletions(-)
diff --git a/submodule.c b/submodule.c
index 422353ccf6cc..1fb753af3066 100644
--- a/submodule.c
+++ b/submodule.c
@@ -278,9 +278,9 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,
static int prepare_submodule_summary(struct rev_info *rev, const char *path,
struct commit *left, struct commit *right,
- int *fast_forward, int *fast_backward)
+ struct commit_list *merge_bases)
{
- struct commit_list *merge_bases, *list;
+ struct commit_list *list;
init_revisions(rev, NULL);
setup_revisions(0, NULL, rev, NULL);
@@ -289,13 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
left->object.flags |= SYMMETRIC_LEFT;
add_pending_object(rev, &left->object, path);
add_pending_object(rev, &right->object, path);
- merge_bases = get_merge_bases(left, right);
- if (merge_bases) {
- if (merge_bases->item == left)
- *fast_forward = 1;
- else if (merge_bases->item == right)
- *fast_backward = 1;
- }
for (list = merge_bases; list; list = list->next) {
list->item->object.flags |= UNINTERESTING;
add_pending_object(rev, &list->item->object,
@@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
strbuf_release(&sb);
}
-void show_submodule_summary(FILE *f, const char *path,
+/* Helper function to display the submodule header line prior to the full
+ * summary output. If it can locate the submodule objects directory it will
+ * attempt to lookup both the left and right commits and put them into the
+ * left and right pointers.
+ */
+static void show_submodule_header(FILE *f, const char *path,
const char *line_prefix,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
- const char *del, const char *add, const char *reset)
+ const char *reset,
+ struct commit **left, struct commit **right,
+ struct commit_list **merge_bases)
{
- struct rev_info rev;
- struct commit *left = NULL, *right = NULL;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
- if (is_null_oid(two))
- message = "(submodule deleted)";
- else if (add_submodule_odb(path))
- message = "(not initialized)";
- else if (is_null_oid(one))
- message = "(new submodule)";
- else if (!(left = lookup_commit_reference(one->hash)) ||
- !(right = lookup_commit_reference(two->hash)))
- message = "(commits not present)";
- else if (prepare_submodule_summary(&rev, path, left, right,
- &fast_forward, &fast_backward))
- message = "(revision walker failed)";
-
if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
fprintf(f, "%sSubmodule %s contains untracked content\n",
line_prefix, path);
@@ -365,11 +350,46 @@ void show_submodule_summary(FILE *f, const char *path,
fprintf(f, "%sSubmodule %s contains modified content\n",
line_prefix, path);
+ if (is_null_oid(one))
+ message = "(new submodule)";
+ else if (is_null_oid(two))
+ message = "(submodule deleted)";
+
+ if (add_submodule_odb(path)) {
+ if (!message)
+ message = "(not initialized)";
+ goto output_header;
+ }
+
+ /*
+ * Attempt to lookup the commit references, and determine if this is
+ * a fast forward or fast backwards update.
+ */
+ *left = lookup_commit_reference(one->hash);
+ *right = lookup_commit_reference(two->hash);
+
+ /*
+ * Warn about missing commits in the submodule project, but only if
+ * they aren't null.
+ */
+ if ((!is_null_oid(one) && !*left) ||
+ (!is_null_oid(two) && !*right))
+ message = "(commits not present)";
+
+ *merge_bases = get_merge_bases(*left, *right);
+ if (*merge_bases) {
+ if ((*merge_bases)->item == *left)
+ fast_forward = 1;
+ else if ((*merge_bases)->item == *right)
+ fast_backward = 1;
+ }
+
if (!oidcmp(one, two)) {
strbuf_release(&sb);
return;
}
+output_header:
strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
find_unique_abbrev(one->hash, DEFAULT_ABBREV));
if (!fast_backward && !fast_forward)
@@ -381,16 +401,45 @@ void show_submodule_summary(FILE *f, const char *path,
strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);
fwrite(sb.buf, sb.len, 1, f);
- if (!message) /* only NULL if we succeeded in setting up the walk */
- print_submodule_summary(&rev, f, line_prefix, del, add, reset);
- if (left)
- clear_commit_marks(left, ~0);
- if (right)
- clear_commit_marks(right, ~0);
-
strbuf_release(&sb);
}
+void show_submodule_summary(FILE *f, const char *path,
+ const char *line_prefix,
+ struct object_id *one, struct object_id *two,
+ unsigned dirty_submodule, const char *meta,
+ const char *del, const char *add, const char *reset)
+{
+ struct rev_info rev;
+ struct commit *left = NULL, *right = NULL;
+ struct commit_list *merge_bases = NULL;
+
+ show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
+ meta, reset, &left, &right, &merge_bases);
+
+ /*
+ * If we don't have both a left and a right pointer, there is no
+ * reason to try and display a summary. The header line should contain
+ * all the information the user needs.
+ */
+ if (!left || !right)
+ goto out;
+
+ /* Treat revision walker failure the same as missing commits */
+ if (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {
+ fprintf(f, "%s(revision walker failed)\n", line_prefix);
+ goto out;
+ }
+
+ print_submodule_summary(&rev, f, line_prefix, del, add, reset);
+
+out:
+ if (merge_bases)
+ free_commit_list(merge_bases);
+ clear_commit_marks(left, ~0);
+ clear_commit_marks(right, ~0);
+}
+
void set_config_fetch_recurse_submodules(int value)
{
config_fetch_recurse_submodules = value;
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v9 8/8] diff: teach diff to display submodule difference with an inline diff
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
` (6 preceding siblings ...)
2016-08-19 23:34 ` [PATCH v9 7/8] submodule: refactor show_submodule_summary with helper function Jacob Keller
@ 2016-08-19 23:34 ` Jacob Keller
2016-08-20 0:02 ` [PATCH v9 0/8] submodule show " Stefan Beller
8 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2016-08-19 23:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stefan Beller, Jeff King, Johannes Sixt,
Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
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>
---
Documentation/diff-config.txt | 9 +-
Documentation/diff-options.txt | 17 +-
diff.c | 31 +-
diff.h | 3 +-
submodule.c | 69 +++
submodule.h | 6 +
t/t4060-diff-submodule-option-diff-format.sh | 749 +++++++++++++++++++++++++++
7 files changed, 863 insertions(+), 21 deletions(-)
create mode 100755 t/t4060-diff-submodule-option-diff-format.sh
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index d5a5b17d5088..0eded24034b5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -122,10 +122,11 @@ diff.suppressBlankEmpty::
diff.submodule::
Specify the format in which differences in submodules are
- shown. The "log" format lists the commits in the range like
- linkgit:git-submodule[1] `summary` does. The "short" format
- format just shows the names of the commits at the beginning
- and end of the range. Defaults to short.
+ shown. The "short" format just shows the names of the commits
+ at the beginning and end of the range. The "log" format lists
+ the commits in the range like linkgit:git-submodule[1] `summary`
+ does. The "diff" format shows an inline diff of the changed
+ contents of the submodule. Defaults to "short".
diff.wordRegex::
A POSIX Extended Regular Expression used to determine what is a "word"
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index cc4342e2034d..7805a0ccadf2 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -210,13 +210,16 @@ any of those replacements occurred.
of the `--diff-filter` option on what the status letters mean.
--submodule[=<format>]::
- Specify how differences in submodules are shown. When `--submodule`
- or `--submodule=log` is given, the 'log' format is used. This format lists
- 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.
+ Specify how differences in submodules are shown. When specifying
+ `--submodule=short` the 'short' format is used. This format just
+ shows the names of the commits at the beginning and end of the range.
+ When `--submodule` or `--submodule=log` is specified, the 'log'
+ format is used. This format lists the commits in the range like
+ linkgit:git-submodule[1] `summary` does. When `--submodule=diff`
+ is specified, the 'diff' format is used. This format shows an
+ inline diff of the changes in the submodule contents between the
+ commit range. Defaults to `diff.submodule` or the 'short' format
+ if the config option is unset.
--color[=<when>]::
Show colored diff.
diff --git a/diff.c b/diff.c
index 16253b191f53..b38d95eb249c 100644
--- a/diff.c
+++ b/diff.c
@@ -135,6 +135,8 @@ static int parse_submodule_params(struct diff_options *options, const char *valu
options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
options->submodule_format = DIFF_SUBMODULE_SHORT;
+ else if (!strcmp(value, "diff"))
+ options->submodule_format = DIFF_SUBMODULE_INLINE_DIFF;
else
return -1;
return 0;
@@ -2300,6 +2302,15 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
+ diff_set_mnemonic_prefix(o, "a/", "b/");
+ if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+ a_prefix = o->b_prefix;
+ b_prefix = o->a_prefix;
+ } else {
+ a_prefix = o->a_prefix;
+ b_prefix = o->b_prefix;
+ }
+
if (o->submodule_format == DIFF_SUBMODULE_LOG &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
@@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,
two->dirty_submodule,
meta, del, add, reset);
return;
+ } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
+ (!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_inline_diff(o->file, one->path ? one->path : two->path,
+ line_prefix,
+ &one->oid, &two->oid,
+ two->dirty_submodule,
+ meta, del, add, reset, o);
+ return;
}
if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
@@ -2318,15 +2340,6 @@ static void builtin_diff(const char *name_a,
textconv_two = get_textconv(two);
}
- diff_set_mnemonic_prefix(o, "a/", "b/");
- if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
- a_prefix = o->b_prefix;
- b_prefix = o->a_prefix;
- } else {
- a_prefix = o->a_prefix;
- b_prefix = o->b_prefix;
- }
-
/* Never use a non-valid filename anywhere if at all possible */
name_a = DIFF_FILE_VALID(one) ? name_a : name_b;
name_b = DIFF_FILE_VALID(two) ? name_b : name_a;
diff --git a/diff.h b/diff.h
index 43b353aea091..ec76a90522ea 100644
--- a/diff.h
+++ b/diff.h
@@ -111,7 +111,8 @@ enum diff_words_type {
enum diff_submodule_format {
DIFF_SUBMODULE_SHORT = 0,
- DIFF_SUBMODULE_LOG
+ DIFF_SUBMODULE_LOG,
+ DIFF_SUBMODULE_INLINE_DIFF
};
struct diff_options {
diff --git a/submodule.c b/submodule.c
index 1fb753af3066..405fa9e4eb32 100644
--- a/submodule.c
+++ b/submodule.c
@@ -440,6 +440,75 @@ void show_submodule_summary(FILE *f, const char *path,
clear_commit_marks(right, ~0);
}
+void show_submodule_inline_diff(FILE *f, const char *path,
+ const char *line_prefix,
+ struct object_id *one, struct object_id *two,
+ unsigned dirty_submodule, const char *meta,
+ const char *del, const char *add, const char *reset,
+ const struct diff_options *o)
+{
+ const struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;
+ struct commit *left = NULL, *right = NULL;
+ struct commit_list *merge_bases = NULL;
+ struct strbuf submodule_dir = STRBUF_INIT;
+ struct child_process cp = CHILD_PROCESS_INIT;
+
+ show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
+ meta, reset, &left, &right, &merge_bases);
+
+ /* We need a valid left and right commit to display a difference */
+ if (!(left || is_null_oid(one)) ||
+ !(right || is_null_oid(two)))
+ goto done;
+
+ if (left)
+ old = one;
+ if (right)
+ new = two;
+
+ fflush(f);
+ cp.git_cmd = 1;
+ cp.dir = path;
+ cp.out = dup(fileno(f));
+ cp.no_stdin = 1;
+
+ /* TODO: other options may need to be passed here. */
+ argv_array_push(&cp.args, "diff");
+ argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
+ if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+ argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
+ o->b_prefix, path);
+ argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",
+ o->a_prefix, path);
+ } else {
+ argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
+ o->a_prefix, path);
+ argv_array_pushf(&cp.args, "--dst-prefix=%s%s/",
+ o->b_prefix, path);
+ }
+ argv_array_push(&cp.args, oid_to_hex(old));
+ /*
+ * If the submodule has modified content, we will diff against the
+ * work tree, under the assumption that the user has asked for the
+ * diff format and wishes to actually see all differences even if they
+ * haven't yet been committed to the submodule yet.
+ */
+ if (!(dirty_submodule & DIRTY_SUBMODULE_MODIFIED))
+ argv_array_push(&cp.args, oid_to_hex(new));
+
+ if (run_command(&cp))
+ fprintf(f, "(diff failed)\n");
+
+done:
+ strbuf_release(&submodule_dir);
+ if (merge_bases)
+ free_commit_list(merge_bases);
+ if (left)
+ clear_commit_marks(left, ~0);
+ if (right)
+ clear_commit_marks(right, ~0);
+}
+
void set_config_fetch_recurse_submodules(int value)
{
config_fetch_recurse_submodules = value;
diff --git a/submodule.h b/submodule.h
index d83df57e24ff..d9e197a948fd 100644
--- a/submodule.h
+++ b/submodule.h
@@ -46,6 +46,12 @@ void show_submodule_summary(FILE *f, const char *path,
struct object_id *one, struct object_id *two,
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset);
+void show_submodule_inline_diff(FILE *f, const char *path,
+ const char *line_prefix,
+ struct object_id *one, struct object_id *two,
+ unsigned dirty_submodule, const char *meta,
+ const char *del, const char *add, const char *reset,
+ const struct diff_options *opt);
void set_config_fetch_recurse_submodules(int value);
void check_for_new_submodule_commits(unsigned char new_sha1[20]);
int fetch_populated_submodules(const struct argv_array *options,
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
new file mode 100755
index 000000000000..7e23b55ea4c5
--- /dev/null
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -0,0 +1,749 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin
+# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding tests)
+# Copyright (c) 2016 Jacob Keller (copy + convert to --submodule=diff)
+#
+
+test_description='Support for diff format verbose submodule difference in git diff
+
+This test tries to verify the sanity of --submodule=diff option of git diff.
+'
+
+. ./test-lib.sh
+
+# Tested non-UTF-8 encoding
+test_encoding="ISO8859-1"
+
+# String "added" in German (translated with Google Translate), encoded in UTF-8,
+# used in sample commit log messages in add_file() function below.
+added=$(printf "hinzugef\303\274gt")
+
+add_file () {
+ (
+ cd "$1" &&
+ shift &&
+ for name
+ do
+ echo "$name" >"$name" &&
+ git add "$name" &&
+ test_tick &&
+ # "git commit -m" would break MinGW, as Windows refuse to pass
+ # $test_encoding encoded parameter to git.
+ echo "Add $name ($added $name)" | iconv -f utf-8 -t $test_encoding |
+ git -c "i18n.commitEncoding=$test_encoding" commit -F -
+ done >/dev/null &&
+ git rev-parse --short --verify HEAD
+ )
+}
+
+commit_file () {
+ test_tick &&
+ git commit "$@" -m "Commit $*" >/dev/null
+}
+
+test_expect_success 'setup repository' '
+ test_create_repo sm1 &&
+ add_file . foo &&
+ head1=$(add_file sm1 foo1 foo2) &&
+ fullhead1=$(git -C sm1 rev-parse --verify HEAD)
+'
+
+test_expect_success 'added submodule' '
+ git add sm1 &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 0000000...$head1 (new submodule)
+ diff --git a/sm1/foo1 b/sm1/foo1
+ new file mode 100644
+ index 0000000..1715acd
+ --- /dev/null
+ +++ b/sm1/foo1
+ @@ -0,0 +1 @@
+ +foo1
+ diff --git a/sm1/foo2 b/sm1/foo2
+ new file mode 100644
+ index 0000000..54b060e
+ --- /dev/null
+ +++ b/sm1/foo2
+ @@ -0,0 +1 @@
+ +foo2
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'added submodule, set diff.submodule' '
+ test_config diff.submodule log &&
+ git add sm1 &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 0000000...$head1 (new submodule)
+ diff --git a/sm1/foo1 b/sm1/foo1
+ new file mode 100644
+ index 0000000..1715acd
+ --- /dev/null
+ +++ b/sm1/foo1
+ @@ -0,0 +1 @@
+ +foo1
+ diff --git a/sm1/foo2 b/sm1/foo2
+ new file mode 100644
+ index 0000000..54b060e
+ --- /dev/null
+ +++ b/sm1/foo2
+ @@ -0,0 +1 @@
+ +foo2
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success '--submodule=short overrides diff.submodule' '
+ test_config diff.submodule log &&
+ git add sm1 &&
+ git diff --submodule=short --cached >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
+ new file mode 160000
+ index 0000000..$head1
+ --- /dev/null
+ +++ b/sm1
+ @@ -0,0 +1 @@
+ +Subproject commit $fullhead1
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'diff.submodule does not affect plumbing' '
+ test_config diff.submodule log &&
+ git diff-index -p HEAD >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
+ new file mode 160000
+ index 0000000..$head1
+ --- /dev/null
+ +++ b/sm1
+ @@ -0,0 +1 @@
+ +Subproject commit $fullhead1
+ EOF
+ test_cmp expected actual
+'
+
+commit_file sm1 &&
+head2=$(add_file sm1 foo3)
+
+test_expect_success 'modified submodule(forward)' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head1..$head2:
+ diff --git a/sm1/foo3 b/sm1/foo3
+ new file mode 100644
+ index 0000000..c1ec6c6
+ --- /dev/null
+ +++ b/sm1/foo3
+ @@ -0,0 +1 @@
+ +foo3
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule(forward)' '
+ git diff --submodule=diff >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head1..$head2:
+ diff --git a/sm1/foo3 b/sm1/foo3
+ new file mode 100644
+ index 0000000..c1ec6c6
+ --- /dev/null
+ +++ b/sm1/foo3
+ @@ -0,0 +1 @@
+ +foo3
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule(forward) --submodule' '
+ git diff --submodule >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head1..$head2:
+ > Add foo3 ($added foo3)
+ EOF
+ test_cmp expected actual
+'
+
+fullhead2=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'modified submodule(forward) --submodule=short' '
+ git diff --submodule=short >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
+ index $head1..$head2 160000
+ --- a/sm1
+ +++ b/sm1
+ @@ -1 +1 @@
+ -Subproject commit $fullhead1
+ +Subproject commit $fullhead2
+ EOF
+ test_cmp expected actual
+'
+
+commit_file sm1 &&
+head3=$(
+ cd sm1 &&
+ git reset --hard HEAD~2 >/dev/null &&
+ git rev-parse --short --verify HEAD
+)
+
+test_expect_success 'modified submodule(backward)' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head2..$head3 (rewind):
+ diff --git a/sm1/foo2 b/sm1/foo2
+ deleted file mode 100644
+ index 54b060e..0000000
+ --- a/sm1/foo2
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo2
+ diff --git a/sm1/foo3 b/sm1/foo3
+ deleted file mode 100644
+ index c1ec6c6..0000000
+ --- a/sm1/foo3
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo3
+ EOF
+ test_cmp expected actual
+'
+
+head4=$(add_file sm1 foo4 foo5)
+test_expect_success 'modified submodule(backward and forward)' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head2...$head4:
+ diff --git a/sm1/foo2 b/sm1/foo2
+ deleted file mode 100644
+ index 54b060e..0000000
+ --- a/sm1/foo2
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo2
+ diff --git a/sm1/foo3 b/sm1/foo3
+ deleted file mode 100644
+ index c1ec6c6..0000000
+ --- a/sm1/foo3
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo3
+ diff --git a/sm1/foo4 b/sm1/foo4
+ new file mode 100644
+ index 0000000..a0016db
+ --- /dev/null
+ +++ b/sm1/foo4
+ @@ -0,0 +1 @@
+ +foo4
+ diff --git a/sm1/foo5 b/sm1/foo5
+ new file mode 100644
+ index 0000000..d6f2413
+ --- /dev/null
+ +++ b/sm1/foo5
+ @@ -0,0 +1 @@
+ +foo5
+ EOF
+ test_cmp expected actual
+'
+
+commit_file sm1 &&
+mv sm1 sm1-bak &&
+echo sm1 >sm1 &&
+head5=$(git hash-object sm1 | cut -c1-7) &&
+git add sm1 &&
+rm -f sm1 &&
+mv sm1-bak sm1
+
+test_expect_success 'typechanged submodule(submodule->blob), --cached' '
+ git diff --submodule=diff --cached >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head4...0000000 (submodule deleted)
+ diff --git a/sm1/foo1 b/sm1/foo1
+ deleted file mode 100644
+ index 1715acd..0000000
+ --- a/sm1/foo1
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo1
+ diff --git a/sm1/foo4 b/sm1/foo4
+ deleted file mode 100644
+ index a0016db..0000000
+ --- a/sm1/foo4
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo4
+ diff --git a/sm1/foo5 b/sm1/foo5
+ deleted file mode 100644
+ index d6f2413..0000000
+ --- a/sm1/foo5
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -foo5
+ diff --git a/sm1 b/sm1
+ new file mode 100644
+ index 0000000..9da5fb8
+ --- /dev/null
+ +++ b/sm1
+ @@ -0,0 +1 @@
+ +sm1
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'typechanged submodule(submodule->blob)' '
+ git diff --submodule=diff >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
+ deleted file mode 100644
+ index 9da5fb8..0000000
+ --- a/sm1
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -sm1
+ Submodule sm1 0000000...$head4 (new submodule)
+ diff --git a/sm1/foo1 b/sm1/foo1
+ new file mode 100644
+ index 0000000..1715acd
+ --- /dev/null
+ +++ b/sm1/foo1
+ @@ -0,0 +1 @@
+ +foo1
+ diff --git a/sm1/foo4 b/sm1/foo4
+ new file mode 100644
+ index 0000000..a0016db
+ --- /dev/null
+ +++ b/sm1/foo4
+ @@ -0,0 +1 @@
+ +foo4
+ diff --git a/sm1/foo5 b/sm1/foo5
+ new file mode 100644
+ index 0000000..d6f2413
+ --- /dev/null
+ +++ b/sm1/foo5
+ @@ -0,0 +1 @@
+ +foo5
+ EOF
+ test_cmp expected actual
+'
+
+rm -rf sm1 &&
+git checkout-index sm1
+test_expect_success 'typechanged submodule(submodule->blob)' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head4...0000000 (submodule deleted)
+ diff --git a/sm1 b/sm1
+ new file mode 100644
+ index 0000000..9da5fb8
+ --- /dev/null
+ +++ b/sm1
+ @@ -0,0 +1 @@
+ +sm1
+ EOF
+ test_cmp expected actual
+'
+
+rm -f sm1 &&
+test_create_repo sm1 &&
+head6=$(add_file sm1 foo6 foo7)
+fullhead6=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'nonexistent commit' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 $head4...$head6 (commits not present)
+ EOF
+ test_cmp expected actual
+'
+
+commit_file
+test_expect_success 'typechanged submodule(blob->submodule)' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
+ deleted file mode 100644
+ index 9da5fb8..0000000
+ --- a/sm1
+ +++ /dev/null
+ @@ -1 +0,0 @@
+ -sm1
+ Submodule sm1 0000000...$head6 (new submodule)
+ diff --git a/sm1/foo6 b/sm1/foo6
+ new file mode 100644
+ index 0000000..462398b
+ --- /dev/null
+ +++ b/sm1/foo6
+ @@ -0,0 +1 @@
+ +foo6
+ diff --git a/sm1/foo7 b/sm1/foo7
+ new file mode 100644
+ index 0000000..6e9262c
+ --- /dev/null
+ +++ b/sm1/foo7
+ @@ -0,0 +1 @@
+ +foo7
+ EOF
+ test_cmp expected actual
+'
+
+commit_file sm1 &&
+test_expect_success 'submodule is up to date' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule contains untracked content' '
+ echo new > sm1/new-file &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains untracked content
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule contains untracked content (untracked ignored)' '
+ git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+test_expect_success 'submodule contains untracked content (dirty ignored)' '
+ git diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+test_expect_success 'submodule contains untracked content (all ignored)' '
+ git diff-index -p --ignore-submodules=all --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+test_expect_success 'submodule contains untracked and modified content' '
+ echo new > sm1/foo6 &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains untracked content
+ Submodule sm1 contains modified content
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+# NOT OK
+test_expect_success 'submodule contains untracked and modified content (untracked ignored)' '
+ echo new > sm1/foo6 &&
+ git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'submodule contains untracked and modified content (dirty ignored)' '
+ echo new > sm1/foo6 &&
+ git diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+test_expect_success 'submodule contains untracked and modified content (all ignored)' '
+ echo new > sm1/foo6 &&
+ git diff-index -p --ignore-submodules --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+test_expect_success 'submodule contains modified content' '
+ rm -f sm1/new-file &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+(cd sm1; git commit -mchange foo6 >/dev/null) &&
+head8=$(cd sm1; git rev-parse --short --verify HEAD) &&
+test_expect_success 'submodule is modified' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9..$head8:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked content' '
+ echo new > sm1/new-file &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains untracked content
+ Submodule sm1 17243c9..$head8:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked content (untracked ignored)' '
+ git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9..$head8:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked content (dirty ignored)' '
+ git diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9..cfce562:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked content (all ignored)' '
+ git diff-index -p --ignore-submodules=all --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+test_expect_success 'modified submodule contains untracked and modified content' '
+ echo modification >> sm1/foo6 &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains untracked content
+ Submodule sm1 contains modified content
+ Submodule sm1 17243c9..cfce562:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..dfda541 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1,2 @@
+ -foo6
+ +new
+ +modification
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked and modified content (untracked ignored)' '
+ echo modification >> sm1/foo6 &&
+ git diff-index -p --ignore-submodules=untracked --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
+ Submodule sm1 17243c9..cfce562:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..e20e2d9 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1,3 @@
+ -foo6
+ +new
+ +modification
+ +modification
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked and modified content (dirty ignored)' '
+ echo modification >> sm1/foo6 &&
+ git diff-index -p --ignore-submodules=dirty --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9..cfce562:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..3e75765 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1 @@
+ -foo6
+ +new
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'modified submodule contains untracked and modified content (all ignored)' '
+ echo modification >> sm1/foo6 &&
+ git diff-index -p --ignore-submodules --submodule=diff HEAD >actual &&
+ ! test -s actual
+'
+
+# NOT OK
+test_expect_success 'modified submodule contains modified content' '
+ rm -f sm1/new-file &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
+ Submodule sm1 17243c9..cfce562:
+ diff --git a/sm1/foo6 b/sm1/foo6
+ index 462398b..ac466ca 100644
+ --- a/sm1/foo6
+ +++ b/sm1/foo6
+ @@ -1 +1,5 @@
+ -foo6
+ +new
+ +modification
+ +modification
+ +modification
+ +modification
+ EOF
+ test_cmp expected actual
+'
+
+rm -rf sm1
+test_expect_success 'deleted submodule' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9...0000000 (submodule deleted)
+ EOF
+ test_cmp expected actual
+'
+
+test_create_repo sm2 &&
+head7=$(add_file sm2 foo8 foo9) &&
+git add sm2
+
+test_expect_success 'multiple submodules' '
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9...0000000 (submodule deleted)
+ Submodule sm2 0000000...a5a65c9 (new submodule)
+ diff --git a/sm2/foo8 b/sm2/foo8
+ new file mode 100644
+ index 0000000..db9916b
+ --- /dev/null
+ +++ b/sm2/foo8
+ @@ -0,0 +1 @@
+ +foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ new file mode 100644
+ index 0000000..9c3b4f6
+ --- /dev/null
+ +++ b/sm2/foo9
+ @@ -0,0 +1 @@
+ +foo9
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'path filter' '
+ git diff-index -p --submodule=diff HEAD sm2 >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm2 0000000...a5a65c9 (new submodule)
+ diff --git a/sm2/foo8 b/sm2/foo8
+ new file mode 100644
+ index 0000000..db9916b
+ --- /dev/null
+ +++ b/sm2/foo8
+ @@ -0,0 +1 @@
+ +foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ new file mode 100644
+ index 0000000..9c3b4f6
+ --- /dev/null
+ +++ b/sm2/foo9
+ @@ -0,0 +1 @@
+ +foo9
+ EOF
+ test_cmp expected actual
+'
+
+commit_file sm2
+test_expect_success 'given commit' '
+ git diff-index -p --submodule=diff HEAD^ >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9...0000000 (submodule deleted)
+ Submodule sm2 0000000...a5a65c9 (new submodule)
+ diff --git a/sm2/foo8 b/sm2/foo8
+ new file mode 100644
+ index 0000000..db9916b
+ --- /dev/null
+ +++ b/sm2/foo8
+ @@ -0,0 +1 @@
+ +foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ new file mode 100644
+ index 0000000..9c3b4f6
+ --- /dev/null
+ +++ b/sm2/foo9
+ @@ -0,0 +1 @@
+ +foo9
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'setup .git file for sm2' '
+ (cd sm2 &&
+ REAL="$(pwd)/../.real" &&
+ mv .git "$REAL"
+ echo "gitdir: $REAL" >.git)
+'
+
+test_expect_success 'diff --submodule=diff with .git file' '
+ git diff --submodule=diff HEAD^ >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 17243c9...0000000 (submodule deleted)
+ Submodule sm2 0000000...a5a65c9 (new submodule)
+ diff --git a/sm2/foo8 b/sm2/foo8
+ new file mode 100644
+ index 0000000..db9916b
+ --- /dev/null
+ +++ b/sm2/foo8
+ @@ -0,0 +1 @@
+ +foo8
+ diff --git a/sm2/foo9 b/sm2/foo9
+ new file mode 100644
+ index 0000000..9c3b4f6
+ --- /dev/null
+ +++ b/sm2/foo9
+ @@ -0,0 +1 @@
+ +foo9
+ EOF
+ test_cmp expected actual
+'
+
+test_done
--
2.10.0.rc0.259.g83512d9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v9 0/8] submodule show inline diff
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
` (7 preceding siblings ...)
2016-08-19 23:34 ` [PATCH v9 8/8] diff: teach diff to display submodule difference with an inline diff Jacob Keller
@ 2016-08-20 0:02 ` Stefan Beller
2016-08-20 6:16 ` Jacob Keller
8 siblings, 1 reply; 12+ messages in thread
From: Stefan Beller @ 2016-08-20 0:02 UTC (permalink / raw)
To: Jacob Keller
Cc: git@vger.kernel.org, Junio C Hamano, Stefan Beller, Jeff King,
Johannes Sixt, Jacob Keller
On Fri, Aug 19, 2016 at 4:34 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> More suggestions from Junio and a few changes to support submodule name
> lookup. Hopefully we're getting close to the goal!
>
> interdiff between v8 and current:
> diff --git c/builtin/rev-list.c w/builtin/rev-list.c
> index 21cde8dd6b31..8479f6ed28aa 100644
> --- c/builtin/rev-list.c
> +++ w/builtin/rev-list.c
> @@ -129,29 +129,29 @@ static void show_commit(struct commit *commit, void *data)
> graph_show_commit_msg(revs->graph, stdout, &buf);
>
> /*
> - * Add a newline after the commit message.
> - *
> - * Usually, this newline produces a blank
> - * padding line between entries, in which case
> - * we need to add graph padding on this line.
> - *
> - * However, the commit message may not end in a
> - * newline. In this case the newline simply
> - * ends the last line of the commit message,
> - * and we don't need any graph output. (This
> - * always happens with CMIT_FMT_ONELINE, and it
> - * happens with CMIT_FMT_USERFORMAT when the
> - * format doesn't explicitly end in a newline.)
> - */
> + * Add a newline after the commit message.
> + *
> + * Usually, this newline produces a blank
> + * padding line between entries, in which case
> + * we need to add graph padding on this line.
> + *
> + * However, the commit message may not end in a
> + * newline. In this case the newline simply
> + * ends the last line of the commit message,
> + * and we don't need any graph output. (This
> + * always happens with CMIT_FMT_ONELINE, and it
> + * happens with CMIT_FMT_USERFORMAT when the
> + * format doesn't explicitly end in a newline.)
> + */
> if (buf.len && buf.buf[buf.len - 1] == '\n')
> graph_show_padding(revs->graph);
> putchar('\n');
> } else {
> /*
> - * If the message buffer is empty, just show
> - * the rest of the graph output for this
> - * commit.
> - */
> + * If the message buffer is empty, just show
> + * the rest of the graph output for this
> + * commit.
> + */
> if (graph_show_remainder(revs->graph))
> putchar('\n');
> if (revs->commit_format == CMIT_FMT_ONELINE)
> diff --git c/cache.h w/cache.h
> index da9f0be67d7b..70428e92d7ed 100644
> --- c/cache.h
> +++ w/cache.h
> @@ -953,24 +953,39 @@ static inline void oidclr(struct object_id *oid)
> #define EMPTY_TREE_SHA1_BIN_LITERAL \
> "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
> "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
> -#define EMPTY_TREE_SHA1_BIN \
> - ((const unsigned char *) EMPTY_TREE_SHA1_BIN_LITERAL)
> +extern const struct object_id empty_tree_oid;
> +#define EMPTY_TREE_SHA1_BIN (empty_tree_oid.hash)
>
> #define EMPTY_BLOB_SHA1_HEX \
> "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391"
> #define EMPTY_BLOB_SHA1_BIN_LITERAL \
> "\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
> "\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
> -#define EMPTY_BLOB_SHA1_BIN \
> - ((const unsigned char *) EMPTY_BLOB_SHA1_BIN_LITERAL)
> +extern const struct object_id empty_blob_oid;
> +#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
>
> -extern const struct object_id empty_tree_oid;
>
> static inline int is_empty_blob_sha1(const unsigned char *sha1)
> {
> return !hashcmp(sha1, EMPTY_BLOB_SHA1_BIN);
> }
>
> +static inline int is_empty_blob_oid(const struct object_id *oid)
> +{
> + return !hashcmp(oid->hash, EMPTY_BLOB_SHA1_BIN);
> +}
> +
> +static inline int is_empty_tree_sha1(const unsigned char *sha1)
> +{
> + return !hashcmp(sha1, EMPTY_TREE_SHA1_BIN);
> +}
> +
> +static inline int is_empty_tree_oid(const struct object_id *oid)
> +{
> + return !hashcmp(oid->hash, EMPTY_TREE_SHA1_BIN);
> +}
> +
> +
> int git_mkstemp(char *path, size_t n, const char *template);
>
> /* set default permissions by passing mode arguments to open(2) */
> diff --git c/diff.h w/diff.h
> index 192c0eedd0ff..ec76a90522ea 100644
> --- c/diff.h
> +++ w/diff.h
> @@ -112,7 +112,7 @@ enum diff_words_type {
> enum diff_submodule_format {
> DIFF_SUBMODULE_SHORT = 0,
> DIFF_SUBMODULE_LOG,
> - DIFF_SUBMODULE_INLINE_DIFF,
> + DIFF_SUBMODULE_INLINE_DIFF
> };
>
> struct diff_options {
> diff --git c/path.c w/path.c
> index 188abfebbafe..081a22c1163c 100644
> --- c/path.c
> +++ w/path.c
> @@ -6,6 +6,7 @@
> #include "string-list.h"
> #include "dir.h"
> #include "worktree.h"
> +#include "submodule-config.h"
>
> static int get_st_mode_bits(const char *path, int *mode)
> {
> @@ -474,6 +475,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
> const char *git_dir;
> struct strbuf git_submodule_common_dir = STRBUF_INIT;
> struct strbuf git_submodule_dir = STRBUF_INIT;
> + const struct submodule *submodule_config;
>
> strbuf_addstr(buf, path);
> strbuf_complete(buf, '/');
> @@ -486,7 +488,16 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
> }
> if (!is_git_directory(buf->buf)) {
> strbuf_reset(buf);
> - strbuf_git_path(buf, "%s/%s", "modules", path);
> + /*
> + * Lookup the submodule name from the config. If that fails
> + * fall back to assuming the path is the name.
> + */
> + submodule_config = submodule_from_path(null_sha1, path);
In case you need to reroll: I'd got with just "sub" as the name for
the config object
(that seems to be used all the time and is shorter)
> + if (submodule_config)
> + strbuf_git_path(buf, "%s/%s", "modules",
> + submodule_config->name);
> + else
I do not think we want to assume the path as the name for the fallback, though.
If `submodule_config == NULL` then
a) the path/name doesn't exist in the given version.
(If null_sha1 is given, HEAD + working tree is assumed, whereas
you could also check for a specific commit of the superproject
with another sha1)
b) or the submodule config cache was not initialized
(missing call to gitmodules_config())
c) There is no c) [at least I never came across another reason for a
NULL return here]
Using the path as the fallback is errorprone (e.g. to b. in the future
and then you get the wrong submodule repository which is shaded by
assuming the path and it is hard to debug later or write forward looking
tests for that now)
Thanks,
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v9 0/8] submodule show inline diff
2016-08-20 0:02 ` [PATCH v9 0/8] submodule show " Stefan Beller
@ 2016-08-20 6:16 ` Jacob Keller
2016-08-22 17:43 ` Stefan Beller
0 siblings, 1 reply; 12+ messages in thread
From: Jacob Keller @ 2016-08-20 6:16 UTC (permalink / raw)
To: Stefan Beller
Cc: Jacob Keller, git@vger.kernel.org, Junio C Hamano, Stefan Beller,
Jeff King, Johannes Sixt
On Fri, Aug 19, 2016 at 5:02 PM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Aug 19, 2016 at 4:34 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
>> - strbuf_git_path(buf, "%s/%s", "modules", path);
>> + /*
>> + * Lookup the submodule name from the config. If that fails
>> + * fall back to assuming the path is the name.
>> + */
>> + submodule_config = submodule_from_path(null_sha1, path);
>
> In case you need to reroll: I'd got with just "sub" as the name for
> the config object
> (that seems to be used all the time and is shorter)
>
Sure.
>
>> + if (submodule_config)
>> + strbuf_git_path(buf, "%s/%s", "modules",
>> + submodule_config->name);
>> + else
>
> I do not think we want to assume the path as the name for the fallback, though.
>
I couldn't think of anything better to do.... There is no error return
flow, so just... leave it as is maybe?
> If `submodule_config == NULL` then
> a) the path/name doesn't exist in the given version.
> (If null_sha1 is given, HEAD + working tree is assumed, whereas
> you could also check for a specific commit of the superproject
> with another sha1)
I can't check for a specific version here because there is no
mechanism to pass in the value we'd check it in... Maybe I need a
separate function for that to check the specific sha1 instead of using
nullsha1? But.. no I don't think that makes sense, we use the current
working tree to get the submodule and then lookup old references in
it... but if we checked an old tree it might be in the wrong path
which does us no good because the name no longer matches? Hmmm
>
> b) or the submodule config cache was not initialized
> (missing call to gitmodules_config())
>
> c) There is no c) [at least I never came across another reason for a
> NULL return here]
>
> Using the path as the fallback is errorprone (e.g. to b. in the future
> and then you get the wrong submodule repository which is shaded by
> assuming the path and it is hard to debug later or write forward looking
> tests for that now)
Sure, but if it doesn't exist we just fail.. so what should I put in
the string? just leave it empty? The function doesn't have an error
return at the moment.
>
> Thanks,
> Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v9 0/8] submodule show inline diff
2016-08-20 6:16 ` Jacob Keller
@ 2016-08-22 17:43 ` Stefan Beller
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Beller @ 2016-08-22 17:43 UTC (permalink / raw)
To: Jacob Keller
Cc: Jacob Keller, git@vger.kernel.org, Junio C Hamano, Stefan Beller,
Jeff King, Johannes Sixt
On Fri, Aug 19, 2016 at 11:16 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>
>>> + if (submodule_config)
>>> + strbuf_git_path(buf, "%s/%s", "modules",
>>> + submodule_config->name);
>>> + else
>>
>> I do not think we want to assume the path as the name for the fallback, though.
>>
>
> I couldn't think of anything better to do.... There is no error return
> flow, so just... leave it as is maybe?
>
>> If `submodule_config == NULL` then
>> a) the path/name doesn't exist in the given version.
>> (If null_sha1 is given, HEAD + working tree is assumed, whereas
>> you could also check for a specific commit of the superproject
>> with another sha1)
>
> I can't check for a specific version here because there is no
> mechanism to pass in the value we'd check it in... Maybe I need a
> separate function for that to check the specific sha1 instead of using
> nullsha1? But.. no I don't think that makes sense, we use the current
> working tree to get the submodule and then lookup old references in
> it... but if we checked an old tree it might be in the wrong path
> which does us no good because the name no longer matches? Hmmm
I agree, null_sha1 is the way to go.
>
>>
>> b) or the submodule config cache was not initialized
>> (missing call to gitmodules_config())
>>
>> c) There is no c) [at least I never came across another reason for a
>> NULL return here]
>>
>> Using the path as the fallback is errorprone (e.g. to b. in the future
>> and then you get the wrong submodule repository which is shaded by
>> assuming the path and it is hard to debug later or write forward looking
>> tests for that now)
>
> Sure, but if it doesn't exist we just fail.. so what should I put in
> the string? just leave it empty? The function doesn't have an error
> return at the moment.
I thought a die(...) would be better instead.
Looking at the callers of do_submodule_path
(which is wrapped via strbuf_git_path_submodule and
git_pathdup_submodule), we end up in
./refs/files-backend.c:
get_packed_ref_cache
resolve_gitlink_ref_recursive
read_loose_refs
other submodule related code:
module_clone
add_submodule_odb
The refs code doesn't have error handling code at the places where
we do the call to submodule path handling, so I think a die(..)
is still ok, as these cases would only happen if your super project
is hosed. e.g. I think to get into this state you'd roughly do this:
git submodule update --init
# make path and name different:
git mv sub-foo sub-bar
# (I think) two ways to hose a repository now:
# a) delete the actual submodule repository
rm -rf .git/modules/sub-foo
# or b) rename the submodule name to break the
# assumption of .git/modules/<name> as the sub repo path.
git config -f .gitmodules --rename-section submodule.sub-foo
submodule.sub-void
# now exercise the refs code:
git status
A wrong path would do no harm in this case; in fact it is better to go
with a non existent wrong name as it then falls back to the error handling
of the refs code.
However, assume the path would exist. (As that happens when you
"swap" two submodules in their path, i.e. when a submodule path becomes
another submodules name.
In that case I think "bad things" may happen to the other submodule?
So after typing all this out, I think a call to die(..) is still the way to go.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-08-22 17:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-19 23:34 [PATCH v9 0/8] submodule show inline diff Jacob Keller
2016-08-19 23:34 ` [PATCH v9 1/8] cache: add empty_tree_oid object and helper function Jacob Keller
2016-08-19 23:34 ` [PATCH v9 2/8] diff.c: remove output_prefix_length field Jacob Keller
2016-08-19 23:34 ` [PATCH v9 3/8] graph: add support for --line-prefix on all graph-aware output Jacob Keller
2016-08-19 23:34 ` [PATCH v9 4/8] diff: prepare for additional submodule formats Jacob Keller
2016-08-19 23:34 ` [PATCH v9 5/8] submodule: allow add_submodule_odb to work even if path is not checked out Jacob Keller
2016-08-19 23:34 ` [PATCH v9 6/8] submodule: convert show_submodule_summary to use struct object_id * Jacob Keller
2016-08-19 23:34 ` [PATCH v9 7/8] submodule: refactor show_submodule_summary with helper function Jacob Keller
2016-08-19 23:34 ` [PATCH v9 8/8] diff: teach diff to display submodule difference with an inline diff Jacob Keller
2016-08-20 0:02 ` [PATCH v9 0/8] submodule show " Stefan Beller
2016-08-20 6:16 ` Jacob Keller
2016-08-22 17:43 ` Stefan Beller
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).