Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> This makes it a little easier for developers to find where things
> where being generated.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230523125000.3674739-5-alex.bennee@linaro.org>
> ---
>  scripts/qapi/gen.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 8f8f784f4a..2ea27ef31c 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -13,6 +13,7 @@
>  
>  from contextlib import contextmanager
>  import os
> +import sys
>  import re
>  from typing import (
>      Dict,
> @@ -162,7 +163,7 @@ def __init__(self, fname: str, blurb: str, pydoc: str):
>  
>      def _top(self) -> str:
>          return mcgen('''
> -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +/* AUTOMATICALLY GENERATED by %(tool)s DO NOT MODIFY */
>  
>  /*
>  %(blurb)s
> @@ -174,6 +175,7 @@ def _top(self) -> str:
>   */
>  
>  ''',
> +                     tool=str(os.path.basename(sys.argv[0])),
>                       blurb=self._blurb, copyright=self._copyright)
>  
>      def _bottom(self) -> str:
> @@ -195,7 +197,9 @@ def _bottom(self) -> str:
>  
>  class QAPIGenTrace(QAPIGen):
>      def _top(self) -> str:
> -        return super()._top() + '# AUTOMATICALLY GENERATED, DO NOT 
> MODIFY\n\n'
> +        return super()._top() + (
> +            '# AUTOMATICALLY GENERATED by '
> +            f"{os.path.basename(sys.argv[0])}, DO NOT MODIFY\n\n" )

Style nitpicks:

* Please don't mix single and double quotes like that.

* You use an f-string to concatenate strings, within a string
  concatenation with operator +.  Let's to mix two ways to do the same
  thing.

* I find the way you break the long line hard to read.

I'd do

        return (super()._top()
                + '# AUTOMATICALLY GENERATED by '
                + os.path.basename(sys.argv[0])
                + ', DO NOT MODIFY\n\n')

>  
>  
>  @contextmanager

With that
Reviewed-by: Markus Armbruster <armbru@redhat.com>