Re: [PATCH] fix show_entry() in tree-diff.c for TREE_IN_RECURSIVE

Subsystems: the rest

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] fix show_entry() in tree-diff.c for TREE_IN_RECURSIVE

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:57

Junio C Hamano [off-list ref] writes:
"Nick Edelen" [off-list ref] writes:
...
quoted
this seems like how it should act: trees are shown under this option for
changes,...
Please show a sample input, the output you expect and the output from the
current code, to illustrate the alleged breakage better.
...
I think the output from the code after your change is more consistent, but
I somehow suspect that this might break people's script, like gitweb,
rather badly, if they depended on the existing behaviour.
That is, something like the attached patch.

I cannot afford to do this to everybody, but you seem to be new to the
community, and every once in a while teaching by showing example is the
best way to do things, so...

-- >8 --
From: Nick Edelen <redacted>
Subject: [PATCH] diff-tree -r -t: include added/removed directories in the output

We used to include only the modified and typechanged directories
in the ouptut, but for consistency's sake, we should also include
added and removed ones as well.

This makes the output more consistent, but it may break existing scripts
that expect to see the current output which has long been the established
behaviour.

Signed-off-by: Nick Edelen <redacted>
Signed-off-by: Junio C Hamano <redacted>
---

 * This should apply at least down to 1.6.0 series, if not earlier.

 t/t4037-diff-r-t-dirs.sh |   53 ++++++++++++++++++++++++++++++++++++++++++++++
 tree-diff.c              |    6 +++++
 2 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/t/t4037-diff-r-t-dirs.sh b/t/t4037-diff-r-t-dirs.sh
new file mode 100755
index 0000000..f5ce3b2
--- /dev/null
+++ b/t/t4037-diff-r-t-dirs.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+test_description='diff -r -t shows directory additions and deletions'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	mkdir dc dr dt &&
+	>dc/1 &&
+	>dr/2 &&
+	>dt/3 &&
+	>fc &&
+	>fr &&
+	>ft &&
+	git add . &&
+	test_tick &&
+	git commit -m initial &&
+
+	rm -fr dt dr ft fr &&
+	mkdir da ft &&
+	for p in dc/1 da/4 dt ft/5 fc
+	do
+		echo hello >$p || exit
+	done &&
+	git add -u &&
+	git add . &&
+	test_tick &&
+	git commit -m second
+'
+
+cat >expect <<\EOF
+A	da
+A	da/4
+M	dc
+M	dc/1
+D	dr
+D	dr/2
+A	dt
+D	dt
+D	dt/3
+M	fc
+D	fr
+D	ft
+A	ft
+A	ft/5
+EOF
+
+test_expect_success verify '
+	git diff-tree -r -t --name-status HEAD^ HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_done
diff --git a/tree-diff.c b/tree-diff.c
index 9f67af6..c83a8da 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -233,6 +233,12 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 		if (!tree || type != OBJ_TREE)
 			die("corrupt tree sha %s", sha1_to_hex(sha1));
 
+		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
+			newbase[baselen + pathlen] = 0;
+			opt->add_remove(opt, *prefix, mode, sha1, newbase);
+			newbase[baselen + pathlen] = '/';
+		}
+
 		init_tree_desc(&inner, tree, size);
 		show_tree(opt, prefix, &inner, newbase, baselen + 1 + pathlen);
 

Re: [PATCH] fix show_entry() in tree-diff.c for TREE_IN_RECURSIVE

From: Nick Edelen <hidden>
Date: 2016-06-15 22:46:57

ah, right.  so next time I should add a test to demonstrate and verify
the external effect?  admittedly part of the reason I hadn't showed
any example output was because I changed this from an internal
perspective.  I'll definitely do that next time though.  thanks for
the example!  (btw, what is the numerical naming convention for
tests?)

you know if no one really liked it as an output we could keep this
change for internal consistency's sake, and filter our added/removed
dirs in the outputing, or even diff_addremove.  seems a bit hackish
though...

(also, do you want me to resubmit?  because you seem to have taken care of it)

 - Nick

On Sun, Jun 14, 2009 at 2:05 AM, Junio C Hamano[off-list ref] wrote:
quoted hunk
Junio C Hamano [off-list ref] writes:
quoted
"Nick Edelen" [off-list ref] writes:
...
quoted
this seems like how it should act: trees are shown under this option for
changes,...
Please show a sample input, the output you expect and the output from the
current code, to illustrate the alleged breakage better.
...
I think the output from the code after your change is more consistent, but
I somehow suspect that this might break people's script, like gitweb,
rather badly, if they depended on the existing behaviour.
That is, something like the attached patch.

I cannot afford to do this to everybody, but you seem to be new to the
community, and every once in a while teaching by showing example is the
best way to do things, so...

-- >8 --
From: Nick Edelen <redacted>
Subject: [PATCH] diff-tree -r -t: include added/removed directories in the output

We used to include only the modified and typechanged directories
in the ouptut, but for consistency's sake, we should also include
added and removed ones as well.

This makes the output more consistent, but it may break existing scripts
that expect to see the current output which has long been the established
behaviour.

Signed-off-by: Nick Edelen <redacted>
Signed-off-by: Junio C Hamano <redacted>
---

 * This should apply at least down to 1.6.0 series, if not earlier.

 t/t4037-diff-r-t-dirs.sh |   53 ++++++++++++++++++++++++++++++++++++++++++++++
 tree-diff.c              |    6 +++++
 2 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/t/t4037-diff-r-t-dirs.sh b/t/t4037-diff-r-t-dirs.sh
new file mode 100755
index 0000000..f5ce3b2
--- /dev/null
+++ b/t/t4037-diff-r-t-dirs.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+test_description='diff -r -t shows directory additions and deletions'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+       mkdir dc dr dt &&
+       >dc/1 &&
+       >dr/2 &&
+       >dt/3 &&
+       >fc &&
+       >fr &&
+       >ft &&
+       git add . &&
+       test_tick &&
+       git commit -m initial &&
+
+       rm -fr dt dr ft fr &&
+       mkdir da ft &&
+       for p in dc/1 da/4 dt ft/5 fc
+       do
+               echo hello >$p || exit
+       done &&
+       git add -u &&
+       git add . &&
+       test_tick &&
+       git commit -m second
+'
+
+cat >expect <<\EOF
+A      da
+A      da/4
+M      dc
+M      dc/1
+D      dr
+D      dr/2
+A      dt
+D      dt
+D      dt/3
+M      fc
+D      fr
+D      ft
+A      ft
+A      ft/5
+EOF
+
+test_expect_success verify '
+       git diff-tree -r -t --name-status HEAD^ HEAD >actual &&
+       test_cmp expect actual
+'
+
+test_done
diff --git a/tree-diff.c b/tree-diff.c
index 9f67af6..c83a8da 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -233,6 +233,12 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
               if (!tree || type != OBJ_TREE)
                       die("corrupt tree sha %s", sha1_to_hex(sha1));

+               if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
+                       newbase[baselen + pathlen] = 0;
+                       opt->add_remove(opt, *prefix, mode, sha1, newbase);
+                       newbase[baselen + pathlen] = '/';
+               }
+
               init_tree_desc(&inner, tree, size);
               show_tree(opt, prefix, &inner, newbase, baselen + 1 + pathlen);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help