Re: [PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-28 Thread Thomas Braun
> Junio C Hamano  hat am 22. November 2018 um 02:34 
> geschrieben:
> 
> 
> Thomas Braun  writes:
> 
> > The -S  option of log looks for differences that changes the
> > number of occurrences of the specified string (i.e. addition/deletion)
> > in a file.
> 
> s/-S /-S/ and
> s/the specified string/the specified block of text/ would make it
> more in line with how Documentation/gitdiffcore.txt explains it.
> The original discussion from early 2017 also explains with a pointer
> why the primary mode of -S is not  but is .

Thanks for the pointer. I've updated the commit message.
 
> > diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> > index 42cc8afd8b..d430f6f2f9 100755
> > --- a/t/t4209-log-pickaxe.sh
> > +++ b/t/t4209-log-pickaxe.sh
> > @@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files 
> > with textconv filter' '
> > test_cmp actual expected
> >  '
> >  
> > +test_expect_success 'log -S looks into binary files' '
> > +   rm -rf .git &&
> > +   git init &&
> 
> Same comment as the one for 1/2 applies here.

Fixed as well.

> > +   printf "a\0b" >data.bin &&
> > +   git add data.bin &&
> > +   git commit -m "message" &&
> > +   git log -S a >actual &&
> > +   git log >expected &&
> > +   test_cmp actual expected
> > +'
> > +
> >  test_done
> 
> Other than these, I think both patches look sensible.  Thanks for
> resurrecting the old topic and reigniting it.
>


Re: [PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-28 Thread Thomas Braun
> Ævar Arnfjörð Bjarmason  hat am 22. November 2018 um 10:14 
> geschrieben:
> 
> 
> 
> On Wed, Nov 21 2018, Thomas Braun wrote:
> 
> > The -S  option of log looks for differences that changes the
> > number of occurrences of the specified string (i.e. addition/deletion)
> > in a file.
> >
> > Add a test to ensure that we keep looking into binary files with -S
> > as changing that would break backwards compatibility in unexpected ways.
> >
> > Signed-off-by: Thomas Braun 
> > ---
> >  t/t4209-log-pickaxe.sh | 11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> > index 42cc8afd8b..d430f6f2f9 100755
> > --- a/t/t4209-log-pickaxe.sh
> > +++ b/t/t4209-log-pickaxe.sh
> > @@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files 
> > with textconv filter' '
> > test_cmp actual expected
> >  '
> >
> > +test_expect_success 'log -S looks into binary files' '
> > +   rm -rf .git &&
> > +   git init &&
> > +   printf "a\0b" >data.bin &&
> > +   git add data.bin &&
> > +   git commit -m "message" &&
> > +   git log -S a >actual &&
> > +   git log >expected &&
> > +   test_cmp actual expected
> > +'
> > +
> >  test_done
> 
> This should just be part of 1/2 since the behavior is changed there &
> the commit message should describe both cases.

My reasoning was that this is a separate test which does not fit in with the 
other part.
But I'm happy in folding both into one patch. Done.


Re: [PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-23 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason  writes:

> On Wed, Nov 21 2018, Thomas Braun wrote:
>
>> The -S  option of log looks for differences that changes the
>> number of occurrences of the specified string (i.e. addition/deletion)
>> in a file.
>>
> ...
> This should just be part of 1/2 since the behavior is changed there &
> the commit message should describe both cases.

Sensible suggestion.  Thanks.


Re: [PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-22 Thread Ævar Arnfjörð Bjarmason


On Wed, Nov 21 2018, Thomas Braun wrote:

> The -S  option of log looks for differences that changes the
> number of occurrences of the specified string (i.e. addition/deletion)
> in a file.
>
> Add a test to ensure that we keep looking into binary files with -S
> as changing that would break backwards compatibility in unexpected ways.
>
> Signed-off-by: Thomas Braun 
> ---
>  t/t4209-log-pickaxe.sh | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> index 42cc8afd8b..d430f6f2f9 100755
> --- a/t/t4209-log-pickaxe.sh
> +++ b/t/t4209-log-pickaxe.sh
> @@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files with 
> textconv filter' '
>   test_cmp actual expected
>  '
>
> +test_expect_success 'log -S looks into binary files' '
> + rm -rf .git &&
> + git init &&
> + printf "a\0b" >data.bin &&
> + git add data.bin &&
> + git commit -m "message" &&
> + git log -S a >actual &&
> + git log >expected &&
> + test_cmp actual expected
> +'
> +
>  test_done

This should just be part of 1/2 since the behavior is changed there &
the commit message should describe both cases.


Re: [PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-21 Thread Junio C Hamano
Thomas Braun  writes:

> The -S  option of log looks for differences that changes the
> number of occurrences of the specified string (i.e. addition/deletion)
> in a file.

s/-S /-S/ and
s/the specified string/the specified block of text/ would make it
more in line with how Documentation/gitdiffcore.txt explains it.
The original discussion from early 2017 also explains with a pointer
why the primary mode of -S is not  but is .

> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> index 42cc8afd8b..d430f6f2f9 100755
> --- a/t/t4209-log-pickaxe.sh
> +++ b/t/t4209-log-pickaxe.sh
> @@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files with 
> textconv filter' '
>   test_cmp actual expected
>  '
>  
> +test_expect_success 'log -S looks into binary files' '
> + rm -rf .git &&
> + git init &&

Same comment as the one for 1/2 applies here.

> + printf "a\0b" >data.bin &&
> + git add data.bin &&
> + git commit -m "message" &&
> + git log -S a >actual &&
> + git log >expected &&
> + test_cmp actual expected
> +'
> +
>  test_done

Other than these, I think both patches look sensible.  Thanks for
resurrecting the old topic and reigniting it.


[PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-21 Thread Thomas Braun
The -S  option of log looks for differences that changes the
number of occurrences of the specified string (i.e. addition/deletion)
in a file.

Add a test to ensure that we keep looking into binary files with -S
as changing that would break backwards compatibility in unexpected ways.

Signed-off-by: Thomas Braun 
---
 t/t4209-log-pickaxe.sh | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index 42cc8afd8b..d430f6f2f9 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -128,4 +128,15 @@ test_expect_success 'log -G looks into binary files with 
textconv filter' '
test_cmp actual expected
 '
 
+test_expect_success 'log -S looks into binary files' '
+   rm -rf .git &&
+   git init &&
+   printf "a\0b" >data.bin &&
+   git add data.bin &&
+   git commit -m "message" &&
+   git log -S a >actual &&
+   git log >expected &&
+   test_cmp actual expected
+'
+
 test_done
-- 
2.19.0.271.gfe8321ec05.dirty