[libcamera-devel] utils: checkstyle.py: Add Doxygen formatter

Message ID 20190703211358.2362-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 91a65e9ee655d7919972cb50811b417c97be95a5
Headers show
Series
  • [libcamera-devel] utils: checkstyle.py: Add Doxygen formatter
Related show

Commit Message

Laurent Pinchart July 3, 2019, 9:13 p.m. UTC
Add a formatter for doxygen comments. In its initial implementation the
formatter ensures that the first word of a \return statement starts with
an uppercase letter.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 utils/checkstyle.py | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Niklas Söderlund July 3, 2019, 10:59 p.m. UTC | #1
Hi Laurent,

Thanks for your work.

On 2019-07-04 00:13:58 +0300, Laurent Pinchart wrote:
> Add a formatter for doxygen comments. In its initial implementation the
> formatter ensures that the first word of a \return statement starts with
> an uppercase letter.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Regexp and lambda function, reviewing is fun!

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  utils/checkstyle.py | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index bc631d405007..fab4b116d2ff 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -350,6 +350,34 @@ class CLangFormatter(Formatter):
>          return ret.stdout.decode('utf-8')
>  
>  
> +class DoxygenFormatter(Formatter):
> +    patterns = ('*.c', '*.cpp')
> +
> +    return_regex = re.compile(' +\\* +\\\\return +[a-z]')
> +
> +    @classmethod
> +    def format(cls, filename, data):
> +        lines = []
> +        in_doxygen = False
> +
> +        for line in data.split('\n'):
> +            if line.find('/**') != -1:
> +                in_doxygen = True
> +
> +            if not in_doxygen:
> +                lines.append(line)
> +                continue
> +
> +            line = cls.return_regex.sub(lambda m: m.group(0)[:-1] + m.group(0)[-1].upper(), line)
> +
> +            if line.find('*/') != -1:
> +                in_doxygen = False
> +
> +            lines.append(line)
> +
> +        return '\n'.join(lines)
> +
> +
>  class StripTrailingSpaceFormatter(Formatter):
>      patterns = ('*.c', '*.cpp', '*.h', '*.py', 'meson.build')
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham July 4, 2019, 9:39 a.m. UTC | #2
Hi Laurent,

On 03/07/2019 22:13, Laurent Pinchart wrote:
> Add a formatter for doxygen comments. In its initial implementation the
> formatter ensures that the first word of a \return statement starts with
> an uppercase letter.

Thanks this looks good, I initially thought that the '/**' and '*/'
should be regexes to validate that they are '^/**$' and '^ */$', but
then I realised that I think we could have indented doxygen statements.

So this looks good.

I ran pep8 on your patch, and while pep8 reports quite a few issues on
checkstyle.py, none of them are in your hunk.

So then I wrote a patch to add a pep8 checker, to, well, - check itself.

--

Anyway,

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  utils/checkstyle.py | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index bc631d405007..fab4b116d2ff 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -350,6 +350,34 @@ class CLangFormatter(Formatter):
>          return ret.stdout.decode('utf-8')
>  
>  
> +class DoxygenFormatter(Formatter):
> +    patterns = ('*.c', '*.cpp')
> +
> +    return_regex = re.compile(' +\\* +\\\\return +[a-z]')
> +
> +    @classmethod
> +    def format(cls, filename, data):
> +        lines = []
> +        in_doxygen = False
> +
> +        for line in data.split('\n'):
> +            if line.find('/**') != -1:
> +                in_doxygen = True
> +
> +            if not in_doxygen:
> +                lines.append(line)
> +                continue
> +
> +            line = cls.return_regex.sub(lambda m: m.group(0)[:-1] + m.group(0)[-1].upper(), line)
> +
> +            if line.find('*/') != -1:
> +                in_doxygen = False
> +
> +            lines.append(line)
> +
> +        return '\n'.join(lines)
> +
> +
>  class StripTrailingSpaceFormatter(Formatter):
>      patterns = ('*.c', '*.cpp', '*.h', '*.py', 'meson.build')
>  
>

Patch

diff --git a/utils/checkstyle.py b/utils/checkstyle.py
index bc631d405007..fab4b116d2ff 100755
--- a/utils/checkstyle.py
+++ b/utils/checkstyle.py
@@ -350,6 +350,34 @@  class CLangFormatter(Formatter):
         return ret.stdout.decode('utf-8')
 
 
+class DoxygenFormatter(Formatter):
+    patterns = ('*.c', '*.cpp')
+
+    return_regex = re.compile(' +\\* +\\\\return +[a-z]')
+
+    @classmethod
+    def format(cls, filename, data):
+        lines = []
+        in_doxygen = False
+
+        for line in data.split('\n'):
+            if line.find('/**') != -1:
+                in_doxygen = True
+
+            if not in_doxygen:
+                lines.append(line)
+                continue
+
+            line = cls.return_regex.sub(lambda m: m.group(0)[:-1] + m.group(0)[-1].upper(), line)
+
+            if line.find('*/') != -1:
+                in_doxygen = False
+
+            lines.append(line)
+
+        return '\n'.join(lines)
+
+
 class StripTrailingSpaceFormatter(Formatter):
     patterns = ('*.c', '*.cpp', '*.h', '*.py', 'meson.build')