Re: [PATCH 1/6] t4051: add test for comments preceding function lines

From: René Scharfe <l.s.r@web.de>
Date: Nov 19, 2017, 10:02 UTC
Message-ID: <bde81d14-a955-4ea4-5799-2a95511d8215@web.de>
In-Reply-To: <CAPig+cQTzBYLqHw5TRYFajTK2Uq2fE=q=h=b=VA43zdJs0_CMA@mail.gmail.com>

Am 19.11.2017 um 02:18 schrieb Eric Sunshine:

Show 16 quoted lines

> On Sat, Nov 18, 2017 at 1:04 PM, René Scharfe <l.s.r@web.de> wrote:
>> When showing function context it would be helpful to show comments
>> immediately before declarations, as they are most likely relevant.  Add
>> a test for that.
>>
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>> ---
>> diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
>> @@ -85,6 +85,10 @@ test_expect_success 'setup' '
>>> +test_expect_failure ' context includes comment' '
>> +       grep "^ .*Hello comment" changed_hello.diff
>> +'
> 
> This shows only that the the "* Hello comment." line is in the context
> but says nothing about the entire comment block being included (which
> patch 3/6 implements). Perhaps the test could be more thorough?

You say that as if it was a bad thing. That line represents 33% of your suggested full-comment metric, i.e. more than nothing. But more importantly it's the actual comment payload. The leading "/*" line is included as a consequence of the employed heuristic, but a more refined one might omit it as it doesn't actually contain any comment.

Not sure if we actually want to tweak our heuristic further, but the test is meant to check only if the interesting part is present and give leeway regarding other implementation details.

Show 12 quoted lines

> 
>> diff --git a/t/t4051/hello.c b/t/t4051/hello.c
>> index 63b1a1e4ef..73e767e178 100644
>> --- a/t/t4051/hello.c
>> +++ b/t/t4051/hello.c
>> @@ -1,4 +1,7 @@
>>
>> +/*
>> + * Hello comment.
>> + */
>>   static void hello(void)        // Begin of hello
>>   {