| Submitter | Alexander Fomin |
|---|---|
| Date | April 4, 2017, 10:21 p.m. |
| Message ID | <373acf7da1a621607a1c.1491344482@devvm2125.lla2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/19962/ |
| State | Changes Requested |
| Headers | show |
Comments
On 4/4/17 11:21 PM, Alexander Fomin wrote: > # HG changeset patch > # User Alexander Fomin <afomin@fb.com> > # Date 1491335167 25200 > # Tue Apr 04 12:46:07 2017 -0700 > # Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016 > # Parent 27515c7e38db9d93e18b7df13149da7c0d88eeb2 > tests: add tests for --binary option in Git mode I think we should keep git uncapitalized here. The code in patch 2 in this series looks good to me. This new test is short enough that I'd fold it into the previous patch. > This patch adds some tests to verify correct --binary option behaviour in Git > mode (issue5510). > > diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t > --- a/tests/test-diff-binary-file.t > +++ b/tests/test-diff-binary-file.t > @@ -107,4 +107,28 @@ > \ No newline at end of file > +\x00\x01\x02\x03 (esc) Remember to add a comment (non-indented line here) with a quick summary of the test. The test itself looks good. > > + $ hg diff --no-binary -r 0 -r 1 > + diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin > + Binary file binfile.bin has changed > + > + $ hg diff --git --no-binary -r 0 -r 1 > + diff --git a/binfile.bin b/binfile.bin > + Binary file binfile.bin has changed > + > + $ hg diff --git --binary -r 0 -r 1 > + diff --git a/binfile.bin b/binfile.bin > + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 > + GIT binary patch > + literal 5 > + Mc${NkWMbw50018V5dZ)H > + > + > + $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1 > + diff --git a/binfile.bin b/binfile.bin > + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 > + GIT binary patch > + literal 5 > + Mc${NkWMbw50018V5dZ)H > + > + > $ cd .. >
On Wed, 5 Apr 2017 15:37:46 +0100, Ryan McElroy wrote: > On 4/4/17 11:21 PM, Alexander Fomin wrote: > > # HG changeset patch > > # User Alexander Fomin <afomin@fb.com> > > # Date 1491335167 25200 > > # Tue Apr 04 12:46:07 2017 -0700 > > # Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016 > > # Parent 27515c7e38db9d93e18b7df13149da7c0d88eeb2 > > tests: add tests for --binary option in Git mode > > I think we should keep git uncapitalized here. > > The code in patch 2 in this series looks good to me. This new test is > short enough that I'd fold it into the previous patch. Yeah, I was about to do that. Can you resend the patches up to here? > > + $ hg diff --no-binary -r 0 -r 1 > > + diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin > > + Binary file binfile.bin has changed > > + > > + $ hg diff --git --no-binary -r 0 -r 1 > > + diff --git a/binfile.bin b/binfile.bin > > + Binary file binfile.bin has changed > > + > > + $ hg diff --git --binary -r 0 -r 1 > > + diff --git a/binfile.bin b/binfile.bin > > + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 > > + GIT binary patch > > + literal 5 > > + Mc${NkWMbw50018V5dZ)H > > + > > + > > + $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1 > > + diff --git a/binfile.bin b/binfile.bin > > + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 > > + GIT binary patch > > + literal 5 > > + Mc${NkWMbw50018V5dZ)H If both --binary and --text are explicitly set, which one will win? Can you add a test about it? I guess text diff will be displayed, which seems correct.
On 05/04/2017, 07:54, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote: > > On Wed, 5 Apr 2017 15:37:46 +0100, Ryan McElroy wrote: > > On 4/4/17 11:21 PM, Alexander Fomin wrote: > > > # HG changeset patch > > > # User Alexander Fomin <afomin@fb.com> > > > # Date 1491335167 25200 > > > # Tue Apr 04 12:46:07 2017 -0700 > > > # Node ID 373acf7da1a621607a1cc062b96ad67a42a7e016 > > > # Parent 27515c7e38db9d93e18b7df13149da7c0d88eeb2 > > > tests: add tests for --binary option in Git mode > > > > I think we should keep git uncapitalized here. > > > > The code in patch 2 in this series looks good to me. This new test is > > short enough that I'd fold it into the previous patch. > > Yeah, I was about to do that. > > Can you resend the patches up to here? > Yes, I’ll merge the second and third patches into one. > > > + $ hg diff --no-binary -r 0 -r 1 > > > + diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin > > > + Binary file binfile.bin has changed > > > + > > > + $ hg diff --git --no-binary -r 0 -r 1 > > > + diff --git a/binfile.bin b/binfile.bin > > > + Binary file binfile.bin has changed > > > + > > > + $ hg diff --git --binary -r 0 -r 1 > > > + diff --git a/binfile.bin b/binfile.bin > > > + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 > > > + GIT binary patch > > > + literal 5 > > > + Mc${NkWMbw50018V5dZ)H > > > + > > > + > > > + $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1 > > > + diff --git a/binfile.bin b/binfile.bin > > > + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 > > > + GIT binary patch > > > + literal 5 > > > + Mc${NkWMbw50018V5dZ)H > > If both --binary and --text are explicitly set, which one will win? Can > you add a test about it? I guess text diff will be displayed, which seems > correct. OK, I’ll add a testcase.
Patch
diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t --- a/tests/test-diff-binary-file.t +++ b/tests/test-diff-binary-file.t @@ -107,4 +107,28 @@ \ No newline at end of file +\x00\x01\x02\x03 (esc) + $ hg diff --no-binary -r 0 -r 1 + diff -r fb45f71337ad -r 9ca112d1a3c1 binfile.bin + Binary file binfile.bin has changed + + $ hg diff --git --no-binary -r 0 -r 1 + diff --git a/binfile.bin b/binfile.bin + Binary file binfile.bin has changed + + $ hg diff --git --binary -r 0 -r 1 + diff --git a/binfile.bin b/binfile.bin + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 + GIT binary patch + literal 5 + Mc${NkWMbw50018V5dZ)H + + + $ hg diff --git --binary --config diff.nobinary=True -r 0 -r 1 + diff --git a/binfile.bin b/binfile.bin + index eaf36c1daccfdf325514461cd1a2ffbc139b5464..ba71a782e93f3fb63a428383706065e3ec2828e9 + GIT binary patch + literal 5 + Mc${NkWMbw50018V5dZ)H + + $ cd ..