[edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

Laszlo Ersek posted 1 patch 4 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20200116184929.18020-1-lersek@redhat.com
BaseTools/Conf/gitattributes | 2 ++
1 file changed, 2 insertions(+)
[edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Laszlo Ersek 4 years, 3 months ago
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 *.h and
*.c 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 *.h and *.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.

Thus, mark our *.h and *.c files 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>
---
 BaseTools/Conf/gitattributes | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Conf/gitattributes b/BaseTools/Conf/gitattributes
index 58b93e9d4c27..8b8b4b92105b 100644
--- a/BaseTools/Conf/gitattributes
+++ b/BaseTools/Conf/gitattributes
@@ -17,3 +17,5 @@
 *.fdf     diff=ini
 *.fdf.inc diff=ini
 *.inf     diff=ini
+*.h       diff=cpp
+*.c       diff=cpp
-- 
2.19.1.3.g30247aa5d201


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53312): https://edk2.groups.io/g/devel/message/53312
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Leif Lindholm 4 years, 3 months ago
On Thu, Jan 16, 2020 at 19:49:29 +0100, 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 *.h and
> *.c 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 *.h and *.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.
> 
> Thus, mark our *.h and *.c files as belonging to the "cpp" git-diff
> driver.
> 
> This change has a dramatic effect on the following command, for example:
> 
> $ git show -W 2ef0c27cb84c

This looks really good, I didn't realise we weren't actually using
properly C-aware diff by default. And thank you for the perfect level
of background.

However, if doing this change, would we want to apply it to all source
code types supported by BaseTools (i.e. referenced in
build_rule.template)?

That would mean:

.c
.cc
.C
.CC
.cpp
.Cpp
.CPP
.h
.H

and possibly

.act
.aslc
.vfr
.Vfr
.VFR

/
    Leif

> 
> 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>
> ---
>  BaseTools/Conf/gitattributes | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/BaseTools/Conf/gitattributes b/BaseTools/Conf/gitattributes
> index 58b93e9d4c27..8b8b4b92105b 100644
> --- a/BaseTools/Conf/gitattributes
> +++ b/BaseTools/Conf/gitattributes
> @@ -17,3 +17,5 @@
>  *.fdf     diff=ini
>  *.fdf.inc diff=ini
>  *.inf     diff=ini
> +*.h       diff=cpp
> +*.c       diff=cpp
> -- 
> 2.19.1.3.g30247aa5d201
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53328): https://edk2.groups.io/g/devel/message/53328
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Liming Gao 4 years, 3 months ago
Leif:
  Vfr is not C style source file. It can't be regarded as C source file. 

Laszlo:
   Is there such support for the assembly file, such as .nasm?

Thanks
Liming
-----Original Message-----
From: Leif Lindholm <leif.lindholm@linaro.org> 
Sent: 2020年1月17日 5:55
To: Laszlo Ersek <lersek@redhat.com>
Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

On Thu, Jan 16, 2020 at 19:49:29 +0100, 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 *.h 
> and *.c 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 *.h and *.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.
> 
> Thus, mark our *.h and *.c files as belonging to the "cpp" git-diff 
> driver.
> 
> This change has a dramatic effect on the following command, for example:
> 
> $ git show -W 2ef0c27cb84c

This looks really good, I didn't realise we weren't actually using properly C-aware diff by default. And thank you for the perfect level of background.

However, if doing this change, would we want to apply it to all source code types supported by BaseTools (i.e. referenced in build_rule.template)?

That would mean:

.c
.cc
.C
.CC
.cpp
.Cpp
.CPP
.h
.H

and possibly

.act
.aslc
.vfr
.Vfr
.VFR

/
    Leif

> 
> 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>
> ---
>  BaseTools/Conf/gitattributes | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/BaseTools/Conf/gitattributes 
> b/BaseTools/Conf/gitattributes index 58b93e9d4c27..8b8b4b92105b 100644
> --- a/BaseTools/Conf/gitattributes
> +++ b/BaseTools/Conf/gitattributes
> @@ -17,3 +17,5 @@
>  *.fdf     diff=ini
>  *.fdf.inc diff=ini
>  *.inf     diff=ini
> +*.h       diff=cpp
> +*.c       diff=cpp
> --
> 2.19.1.3.g30247aa5d201
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53329): https://edk2.groups.io/g/devel/message/53329
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Laszlo Ersek 4 years, 3 months ago
On 01/17/20 01:22, Gao, Liming wrote:
> Leif:
>   Vfr is not C style source file. It can't be regarded as C source
>   file.
>
> Laszlo:
>    Is there such support for the assembly file, such as .nasm?

According to the gitattributes(5) manual, there doesn't seem to be a
git-diff driver for assembly files.

However, the default (language-independent) driver seems to handle it
reasonably well. Consider commit f4c898f2b2db
("UefiCpuPkg/CpuExceptionHandlerLib: Fix split lock", 2019-09-20).

$ git show f4c898f2b2db

this displays just the changes, with 3 lines of context, but it already
correctly displays the "HasErrorCode" label in the hunk header:

@@ -184,17 +184,19 @@ HasErrorCode:

Now, if I add "-W":

$ git show -W f4c898f2b2db

then the full text is displayed between "HasErrorCode" and "ErrorCode".

(You can see the entire file with:

$ git show f4c898f2b2db:UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm

)

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53350): https://edk2.groups.io/g/devel/message/53350
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Leif Lindholm 4 years, 3 months ago
Liming,

Hence the "possibly" :)

Although, I agree, grouping it with .aslc (which definitely *is* C),
was a bit confusing.

I couldn't actually find any .act in the tree - are those obsolete, or
do we simply not have any in the tree? (If the latter, should we add
something in order to support simple build tests?)

Regards,

Leif

On Fri, Jan 17, 2020 at 00:22:00 +0000, Gao, Liming wrote:
> Leif:
>   Vfr is not C style source file. It can't be regarded as C source file. 
> 
> Laszlo:
>    Is there such support for the assembly file, such as .nasm?
> 
> Thanks
> Liming
> -----Original Message-----
> From: Leif Lindholm <leif.lindholm@linaro.org> 
> Sent: 2020年1月17日 5:55
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
> 
> On Thu, Jan 16, 2020 at 19:49:29 +0100, 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 *.h 
> > and *.c 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 *.h and *.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.
> > 
> > Thus, mark our *.h and *.c files as belonging to the "cpp" git-diff 
> > driver.
> > 
> > This change has a dramatic effect on the following command, for example:
> > 
> > $ git show -W 2ef0c27cb84c
> 
> This looks really good, I didn't realise we weren't actually using properly C-aware diff by default. And thank you for the perfect level of background.
> 
> However, if doing this change, would we want to apply it to all source code types supported by BaseTools (i.e. referenced in build_rule.template)?
> 
> That would mean:
> 
> .c
> .cc
> .C
> .CC
> .cpp
> .Cpp
> .CPP
> .h
> .H
> 
> and possibly
> 
> .act
> .aslc
> .vfr
> .Vfr
> .VFR
> 
> /
>     Leif
> 
> > 
> > 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>
> > ---
> >  BaseTools/Conf/gitattributes | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/BaseTools/Conf/gitattributes 
> > b/BaseTools/Conf/gitattributes index 58b93e9d4c27..8b8b4b92105b 100644
> > --- a/BaseTools/Conf/gitattributes
> > +++ b/BaseTools/Conf/gitattributes
> > @@ -17,3 +17,5 @@
> >  *.fdf     diff=ini
> >  *.fdf.inc diff=ini
> >  *.inf     diff=ini
> > +*.h       diff=cpp
> > +*.c       diff=cpp
> > --
> > 2.19.1.3.g30247aa5d201
> > 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53337): https://edk2.groups.io/g/devel/message/53337
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Liming Gao 4 years, 3 months ago
Leif:
  .act is same to .aslc. There are no cases in open source. 

Thanks
Liming
-----Original Message-----
From: Leif Lindholm <leif.lindholm@linaro.org> 
Sent: 2020年1月17日 8:51
To: Gao, Liming <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>
Subject: Re: [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

Liming,

Hence the "possibly" :)

Although, I agree, grouping it with .aslc (which definitely *is* C), was a bit confusing.

I couldn't actually find any .act in the tree - are those obsolete, or do we simply not have any in the tree? (If the latter, should we add something in order to support simple build tests?)

Regards,

Leif

On Fri, Jan 17, 2020 at 00:22:00 +0000, Gao, Liming wrote:
> Leif:
>   Vfr is not C style source file. It can't be regarded as C source file. 
> 
> Laszlo:
>    Is there such support for the assembly file, such as .nasm?
> 
> Thanks
> Liming
> -----Original Message-----
> From: Leif Lindholm <leif.lindholm@linaro.org>
> Sent: 2020年1月17日 5:55
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Gao, 
> Liming <liming.gao@intel.com>
> Subject: Re: [PATCH] BaseTools/Conf/gitattributes: fix 
> "--function-context" for *.h and *.c
> 
> On Thu, Jan 16, 2020 at 19:49:29 +0100, 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 
> > *.h and *.c 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 *.h and *.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.
> > 
> > Thus, mark our *.h and *.c files as belonging to the "cpp" git-diff 
> > driver.
> > 
> > This change has a dramatic effect on the following command, for example:
> > 
> > $ git show -W 2ef0c27cb84c
> 
> This looks really good, I didn't realise we weren't actually using properly C-aware diff by default. And thank you for the perfect level of background.
> 
> However, if doing this change, would we want to apply it to all source code types supported by BaseTools (i.e. referenced in build_rule.template)?
> 
> That would mean:
> 
> .c
> .cc
> .C
> .CC
> .cpp
> .Cpp
> .CPP
> .h
> .H
> 
> and possibly
> 
> .act
> .aslc
> .vfr
> .Vfr
> .VFR
> 
> /
>     Leif
> 
> > 
> > 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>
> > ---
> >  BaseTools/Conf/gitattributes | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/BaseTools/Conf/gitattributes 
> > b/BaseTools/Conf/gitattributes index 58b93e9d4c27..8b8b4b92105b 
> > 100644
> > --- a/BaseTools/Conf/gitattributes
> > +++ b/BaseTools/Conf/gitattributes
> > @@ -17,3 +17,5 @@
> >  *.fdf     diff=ini
> >  *.fdf.inc diff=ini
> >  *.inf     diff=ini
> > +*.h       diff=cpp
> > +*.c       diff=cpp
> > --
> > 2.19.1.3.g30247aa5d201
> > 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53336): https://edk2.groups.io/g/devel/message/53336
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Laszlo Ersek 4 years, 3 months ago
On 01/17/20 01:56, Gao, Liming wrote:
> Leif:
>   .act is same to .aslc. There are no cases in open source. 

How about this then:

*.[Aa][Cc][Tt]     diff=cpp
*.[Aa][Ss][Ll][Cc] diff=cpp
*.[CcHh]           diff=cpp
*.[Cc][Cc]         diff=cpp
*.[Cc][Pp][Pp]     diff=cpp

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53352): https://edk2.groups.io/g/devel/message/53352
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Leif Lindholm 4 years, 3 months ago
On Fri, Jan 17, 2020 at 09:30:49 +0100, Laszlo Ersek wrote:
> On 01/17/20 01:56, Gao, Liming wrote:
> > Leif:
> >   .act is same to .aslc. There are no cases in open source. 
> 
> How about this then:
> 
> *.[Aa][Cc][Tt]     diff=cpp
> *.[Aa][Ss][Ll][Cc] diff=cpp
> *.[CcHh]           diff=cpp
> *.[Cc][Cc]         diff=cpp
> *.[Cc][Pp][Pp]     diff=cpp

To be honest, I would rather just see them all listed explicitly.
The above actually adds capitalisation-combos not currently listed in
build_rules.template (like .CpP).

Given how abolutely trivial the fixup for anyone affected would be, I
think we should also start considering deprecating (with warning) and
finally retiring file endings not available in any public trees.
Clearly, that isn't something that needs fixing for this set :)

Best Regards,

Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53362): https://edk2.groups.io/g/devel/message/53362
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Laszlo Ersek 4 years, 3 months ago
On 01/17/20 14:05, Leif Lindholm wrote:
> On Fri, Jan 17, 2020 at 09:30:49 +0100, Laszlo Ersek wrote:
>> On 01/17/20 01:56, Gao, Liming wrote:
>>> Leif:
>>>   .act is same to .aslc. There are no cases in open source. 
>>
>> How about this then:
>>
>> *.[Aa][Cc][Tt]     diff=cpp
>> *.[Aa][Ss][Ll][Cc] diff=cpp
>> *.[CcHh]           diff=cpp
>> *.[Cc][Cc]         diff=cpp
>> *.[Cc][Pp][Pp]     diff=cpp
> 
> To be honest, I would rather just see them all listed explicitly.
> The above actually adds capitalisation-combos not currently listed in
> build_rules.template (like .CpP).

I agree .CpP is non-intuitive, but you previously quoted .Cpp from
"BaseTools/Conf/build_rule.template", which is just as non-intuitive to
me. Who'd *capitalize* a file suffix? (I'd understand *all*-caps.)

So anyway: you suggest to copy the file suffix list verbatim from the
[C-Code-File] and [Acpi-Table-Code-File] sections in
"BaseTools/Conf/build_rule.template". Plus, on top of those, *.h and
*.H. Correct?

Thanks,
Laszlo

> Given how abolutely trivial the fixup for anyone affected would be, I
> think we should also start considering deprecating (with warning) and
> finally retiring file endings not available in any public trees.
> Clearly, that isn't something that needs fixing for this set :)
> 
> Best Regards,
> 
> Leif
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53369): https://edk2.groups.io/g/devel/message/53369
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Leif Lindholm 4 years, 3 months ago
Hi Laszlo,

On Fri, Jan 17, 2020 at 19:09:22 +0100, Laszlo Ersek wrote:
> On 01/17/20 14:05, Leif Lindholm wrote:
> > On Fri, Jan 17, 2020 at 09:30:49 +0100, Laszlo Ersek wrote:
> >> On 01/17/20 01:56, Gao, Liming wrote:
> >>> Leif:
> >>>   .act is same to .aslc. There are no cases in open source. 
> >>
> >> How about this then:
> >>
> >> *.[Aa][Cc][Tt]     diff=cpp
> >> *.[Aa][Ss][Ll][Cc] diff=cpp
> >> *.[CcHh]           diff=cpp
> >> *.[Cc][Cc]         diff=cpp
> >> *.[Cc][Pp][Pp]     diff=cpp
> > 
> > To be honest, I would rather just see them all listed explicitly.
> > The above actually adds capitalisation-combos not currently listed in
> > build_rules.template (like .CpP).
> 
> I agree .CpP is non-intuitive, but you previously quoted .Cpp from
> "BaseTools/Conf/build_rule.template", which is just as non-intuitive to
> me. Who'd *capitalize* a file suffix? (I'd understand *all*-caps.)

I agree, I just included everything explicitly listed today.

> So anyway: you suggest to copy the file suffix list verbatim from the
> [C-Code-File] and [Acpi-Table-Code-File] sections in
> "BaseTools/Conf/build_rule.template". Plus, on top of those, *.h and
> *.H. Correct?

Yes please.

Best Regards,

Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53370): https://edk2.groups.io/g/devel/message/53370
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c
Posted by Liming Gao 4 years, 3 months ago
Laszlo:
  I also agree to handle those file type specified in build_rule.txt. 

Thanks
Liming
-----Original Message-----
From: Leif Lindholm <leif.lindholm@linaro.org> 
Sent: 2020年1月18日 3:37
To: Laszlo Ersek <lersek@redhat.com>
Cc: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>
Subject: Re: [PATCH] BaseTools/Conf/gitattributes: fix "--function-context" for *.h and *.c

Hi Laszlo,

On Fri, Jan 17, 2020 at 19:09:22 +0100, Laszlo Ersek wrote:
> On 01/17/20 14:05, Leif Lindholm wrote:
> > On Fri, Jan 17, 2020 at 09:30:49 +0100, Laszlo Ersek wrote:
> >> On 01/17/20 01:56, Gao, Liming wrote:
> >>> Leif:
> >>>   .act is same to .aslc. There are no cases in open source. 
> >>
> >> How about this then:
> >>
> >> *.[Aa][Cc][Tt]     diff=cpp
> >> *.[Aa][Ss][Ll][Cc] diff=cpp
> >> *.[CcHh]           diff=cpp
> >> *.[Cc][Cc]         diff=cpp
> >> *.[Cc][Pp][Pp]     diff=cpp
> > 
> > To be honest, I would rather just see them all listed explicitly.
> > The above actually adds capitalisation-combos not currently listed 
> > in build_rules.template (like .CpP).
> 
> I agree .CpP is non-intuitive, but you previously quoted .Cpp from 
> "BaseTools/Conf/build_rule.template", which is just as non-intuitive 
> to me. Who'd *capitalize* a file suffix? (I'd understand *all*-caps.)

I agree, I just included everything explicitly listed today.

> So anyway: you suggest to copy the file suffix list verbatim from the 
> [C-Code-File] and [Acpi-Table-Code-File] sections in 
> "BaseTools/Conf/build_rule.template". Plus, on top of those, *.h and 
> *.H. Correct?

Yes please.

Best Regards,

Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53380): https://edk2.groups.io/g/devel/message/53380
Mute This Topic: https://groups.io/mt/69751918/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-