[libcamera-devel] [PATCH 1/2] utils: checkstyle: Add formatter to sort #include statements

[Kieran Bingham]

Hi Laurent,

On 22/03/2020 12:02, Laurent Pinchart wrote:

Ha, I love this :-)

One less thing to worry about in reviews!

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  utils/checkstyle.py | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index 0827a1e6ba0f..5fd63f047781 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -481,6 +481,39 @@ class DoxygenFormatter(Formatter):
>          return '\n'.join(lines)
>  
>  
> +class IncludeOrderFormatter(Formatter):
> +    patterns = ('*.cpp', '*.h')
> +
> +    include_regex = re.compile('^#include ["<]([^">]*)[">]')
> +
> +    @classmethod
> +    def format(cls, filename, data):
> +        lines = []
> +        includes = []
> +
> +        for line in data.split('\n'):
> +            match = IncludeOrderFormatter.include_regex.match(line)
> +            if match:
> +                includes.append((line, match.group(1)))
> +                continue
> +

Maybe it's just me, but this seems somehow quite hard to interpret and I
found it difficult to work out /how/ the code is parsing the blocks.

I'm not yet sure what, but a comment describing what happens somewhere
would be useful for the future me...

(It didn't help that I missed the continue on my first read, so I didn't
realise the code below was only executed for non #include lines).

Maybe a top level comment saying:
 # Parse blocks of #include statements, and output them as a sorted list
 # when we reach a non #include statement.


Perhaps that's verging towards commenting what the code does rather than
why, but now that I 'know' that information, it's easier to parse the
paths of the code to determine the 'how'...


> +            if len(includes):
> +                includes.sort(key=lambda i: i[1])
> +                for include in includes:
> +                    lines.append(include[0])
> +                includes = []
> +
> +            lines.append(line)
> +
> +        if len(includes):
> +            includes.sort(key=lambda i: i[1])
> +            for include in includes:
> +                lines.append(include[0])
> +            includes = []
> +
> +        return '\n'.join(lines)
> +
> +
>  class StripTrailingSpaceFormatter(Formatter):
>      patterns = ('*.c', '*.cpp', '*.h', '*.py', 'meson.build')
>  
> 

-- 
Regards
--
Kieran