Hi Dscho, On Tue, Feb 5, 2019 at 1:48 AM Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > > Hi Elijah, > > On Mon, 4 Feb 2019, Elijah Newren wrote: > > > The combined diff format for merges will only list one filename, even if > > rename or copy detection is active. For example, with raw format one > > might see: > > > > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM describe.c > > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM bar.sh > > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR phooey.c > > > > This doesn't let us know what the original name of bar.sh was in the > > first parent, and doesn't let us know what either of the original names > > of phooey.c were in either of the parents. In contrast, for non-merge > > commits, raw format does provide original filenames (and a rename score > > to boot). In order to also provide original filenames for merge > > commits, add a --combined-all-names option (which must be used with > > either -c or --cc, and is likely only useful with rename or copy > > detection active) so that we can print tab-separated filenames when > > renames are involved. This transforms the above output to: > > > > ::100644 100644 100644 fabadb8 cc95eb0 4866510 MM desc.c desc.c desc.c > > ::100755 100755 100755 52b7a2d 6d1ac04 d2ac7d7 RM foo.sh bar.sh bar.sh > > ::100644 100644 100644 e07d6c5 9042e82 ee91881 RR fooey.c fuey.c phooey.c > > > > Further, in patch format, this changes the from/to headers so that > > instead of just having one "from" header, we get one for each parent. > > For example, instead of having > > > > --- a/phooey.c > > +++ b/phooey.c > > > > we would see > > > > --- a/fooey.c > > --- a/fuey.c > > +++ b/phooey.c > > > > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> > > I do not really see where this needs to have tests with filenames > containing tabs, but your test does create such files. Which makes it > break on filesystems that do not allow tabs in file names, such as > NTFS. I need this to make the test pass again: > > -- snip -- > diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh > index 5bccc323f648..596705985baa 100755 > --- a/t/t4038-diff-combined.sh > +++ b/t/t4038-diff-combined.sh > @@ -435,7 +435,7 @@ test_expect_success 'combine diff gets tree sorting right' ' > test_cmp expect actual > ' > > -test_expect_success 'setup for --combined-with-paths' ' > +test_expect_success FUNNYNAMES 'setup for --combined-with-paths' ' > git branch side1c && > git branch side2c && > git checkout side1c && > @@ -456,7 +456,7 @@ test_expect_success 'setup for --combined-with-paths' ' > git commit > ' > > -test_expect_success '--combined-all-names and --raw' ' > +test_expect_success FUNNYNAMES '--combined-all-names and --raw' ' > cat <<-\EOF >expect && > ::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR "file\twith\ttabs" "i\tam\ttabbed" "fickle\tnaming" > EOF > @@ -465,13 +465,13 @@ test_expect_success '--combined-all-names and --raw' ' > test_cmp expect actual > ' > > -test_expect_success '--combined-all-names and --raw -and -z' ' > +test_expect_success FUNNYNAMES '--combined-all-names and --raw -and -z' ' > printf "0f9645804ebb04cc3eef91f799eb7fb54d70cefb\0::100644 100644 100644 f00c965d8307308469e537302baa73048488f162 088bd5d92c2a8e0203ca8e7e4c2a5c692f6ae3f7 333b9c62519f285e1854830ade0fe1ef1d40ee1b RR\0file\twith\ttabs\0i\tam\ttabbed\0fickle\tnaming\0" >expect && > git diff-tree -c -M --raw --combined-all-names -z HEAD >actual && > test_cmp -a expect actual > ' > > -test_expect_success '--combined-all-names and --cc' ' > +test_expect_success FUNNYNAMES '--combined-all-names and --cc' ' > cat <<-\EOF >expect && > --- "a/file\twith\ttabs" > --- "a/i\tam\ttabbed" > -- snap -- > > But maybe you want to get rid of the funny file names? Or test for them in > a separate, dedicated test case so that we do not have to guard *all* of > your added tests behind FUNNYNAMES? I don't want to get rid of them; the initial reviews of my original patch thought the format was ambiguous and would mishandle files with tabs in them. But yeah, I can definitely add a few testcases without FUNNYNAMES. Sorry for the headache; I'll fix this up. Elijah
