Re: [edk2-devel] [PATCH v2] BaseTools/Conf/gitattributes: fix "--function-context" for C source code

["Laszlo Ersek" <lersek@redhat.com>]

Hi Mike,

On 01/23/20 13:13, Laszlo Ersek wrote:
> Mike,
> 
> can you please ACK this patch while Liming and Bob are away? It's very
> simple.

I intended to write "ping" here... But then I realized I failed to add
you to the "To:" list in the first place, when I asked you to review! :(

Sigh.

So, can you please review this now? :)

Thanks
Laszlo

> On 01/20/20 10:42, Laszlo Ersek wrote:
>> The "--function-context" ("-W") option of git-diff displays the entire
>> body of a modified function, not just small modified hunks within the
>> function. It is useful for reviewers when the code changes to the function
>> are small, but they could affect, or depend on, control flow that is far
>> away in the same function.
>>
>> Of course, the size of the displayed context can be controlled with the
>> "-U" option anyway, but such fixed-size contexts are usually either too
>> small, or too large, in the above scenario.
>>
>> It turns out that "--function-context" does not work correctly for C
>> source files in edk2. In particular, labels for the goto instruction
>> (which the edk2 coding style places in the leftmost column) appear to
>> terminate "--function-context".
>>
>> The "git" utility contains built-in hunk header patterns for the C and C++
>> languages. However, they do not take effect in edk2 because we don't
>> explicitly assign the "cpp" git-diff driver to our C files. The
>> gitattributes(5) manual explains that this is required:
>>
>>>            There are a few built-in patterns to make this easier, and
>>>            tex is one of them, so you do not have to write the above in
>>>            your configuration file (you still need to enable this with
>>>            the attribute mechanism, via .gitattributes). The following
>>>            built in patterns are available:
>>>
>>>            [...]
>>>
>>>            *   cpp suitable for source code in the C and C++
>>>                languages.
>>
>> The key statement is the one in parentheses.
>>
>> Grab the suffix lists from the [C-Code-File] and [Acpi-Table-Code-File]
>> sections of "BaseTools/Conf/build_rule.template", add "*.h" and "*.H", and
>> mark those as belonging to the "cpp" git-diff driver.
>>
>> This change has a dramatic effect on the following command, for example:
>>
>> $ git show -W 2ef0c27cb84c
>>
>> Cc: Bob Feng <bob.c.feng@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     v2:
>>     
>>     - take suffixes from "BaseTools/Conf/build_rule.template" [Leif]
>>     
>>     - replace "*.h" / "*.c" in the commit message with "C source" and
>>       [C-Code-File] / [Acpi-Table-Code-File]
>>     
>>     - Supersedes: <20200116184929.18020-1-lersek@redhat.com>
>>       http://mid.mail-archive.com/20200116184929.18020-1-lersek@redhat.com
>>       https://edk2.groups.io/g/devel/message/53312
>>
>>  BaseTools/Conf/gitattributes | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/BaseTools/Conf/gitattributes b/BaseTools/Conf/gitattributes
>> index 58b93e9d4c27..319e89064133 100644
>> --- a/BaseTools/Conf/gitattributes
>> +++ b/BaseTools/Conf/gitattributes
>> @@ -17,3 +17,14 @@
>>  *.fdf     diff=ini
>>  *.fdf.inc diff=ini
>>  *.inf     diff=ini
>> +*.c       diff=cpp
>> +*.C       diff=cpp
>> +*.cc      diff=cpp
>> +*.CC      diff=cpp
>> +*.cpp     diff=cpp
>> +*.Cpp     diff=cpp
>> +*.CPP     diff=cpp
>> +*.aslc    diff=cpp
>> +*.act     diff=cpp
>> +*.h       diff=cpp
>> +*.H       diff=cpp
>>
>