Re: [Patch 3/3 V2] BaseTools/PatchCheck.py: Update to report error for EFI_D_*

["Zhu, Yonghong" <yonghong.zhu@intel.com>]

Hi Jordan,

If we change regex to r'''DEBUG \s* \( \s* \(\s*. EFI_D_*''', it cannot work because we would use mo.group(1) which report IndexError.
If we change regex to r'''DEBUG \s* \( \s* \(\s*. EFI_D_ ([A-Z_]+)''' , it cannot detect the case "DEBUG ((DEBUG_ERROR | EFI_D_INFO, "xxx", Status));"
So how about keep the original regex ?

Best Regards,
Zhu Yonghong


-----Original Message-----
From: Justen, Jordan L 
Sent: Wednesday, October 19, 2016 2:32 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>
Subject: Re: [Patch 3/3 V2] BaseTools/PatchCheck.py: Update to report error for EFI_D_*

On 2016-10-18 03:16:53, Yonghong Zhu wrote:
> In EDK2, DEBUG_* is recommended to be used instead of EFI_D_*. For new 
> code, they should use DEBUG_* macro.
> 
> Fixes:https://bugzilla.tianocore.org/show_bug.cgi?id=143
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
>  BaseTools/Scripts/PatchCheck.py | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py 
> b/BaseTools/Scripts/PatchCheck.py index 05f8f6e..083438c 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -338,10 +338,16 @@ class GitDiffCheck:
>              lines.append('File: ' + self.hunk_filename)
>          lines.append('Line: ' + line)
>  
>          self.error(*lines)
>  
> +    old_debug_re = \
> +        re.compile(r'''
> +                       DEBUG \s* \( \s* \( .* EFI_D_ ([A-Z_]+)
                                              ^^

I recommend changing .* in the regex to \s*. EFI_D_* is the first parameter. The only thing that might appear is a comment, but that could also appear between the parentheses. This would be an unusual place to put a comment.

Aside from that,

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

> +                   ''',
> +                   re.VERBOSE)
> +
>      def check_added_line(self, line):
>          eol = ''
>          for an_eol in self.line_endings:
>              if line.endswith(an_eol):
>                  eol = an_eol
> @@ -355,10 +361,16 @@ class GitDiffCheck:
>          if '    ' in line:
>              self.added_line_error('Tab character used', line)
>          if len(stripped) < len(line):
>              self.added_line_error('Trailing whitespace found', line)
>  
> +        mo = self.old_debug_re.search(line)
> +        if mo is not None:
> +            self.added_line_error('EFI_D_' + mo.group(1) + ' was used, '
> +                                  'but DEBUG_' + mo.group(1) +
> +                                  ' is now recommended', line)
> +
>      split_diff_re = re.compile(r'''
>                                     (?P<cmd>
>                                         ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
>                                     )
>                                     (?P<index>
> --
> 2.6.1.windows.1
>