Re: [dpdk-dev] [PATCH v2 1/2] devtools: script to check meson indentation of lists

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 26-Apr-21 11:54 AM, Bruce Richardson wrote:
> This is a script to fix up minor formatting issues in meson files.
> It scans for, and can optionally fix, indentation issues and missing
> trailing commas in the lists in meson.build files. It also detects,
> and can fix, multi-line lists where more than one entry appears on a
> line.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

<snip>

> +def split_code_comments(line):
> +    'splits a line into a code part and a comment part, returns (code, comment) tuple'
> +    if line.lstrip().startswith('#'):
> +        return ('', line)
> +    elif '#' in line and '#include' not in line:  # catch 99% of cases, not 100% > +        idx = line.index('#')
> +        while (line[idx - 1].isspace()):
> +            idx -= 1
> +        return line[:idx], line[idx:]


I think this could be simplified with regex:

# find any occurrences of '#' but only if it's not an '#include'
if not re.search(r'#(?!include)', line)
     return line, ''
return line.split('#', maxsplit=1)

> +    else:
> +        return (line, '')
> +
> +
> +def setline(contents, index, value):
> +    'sets the contents[index] to value. Returns the line, along with code and comments part'
> +    line = contents[index] = value
> +    code, comments = split_code_comments(line)
> +    return line, code, comments
> +
> +
> +def check_indentation(filename, contents):
> +    '''check that a list or files() is correctly indented'''
> +    infiles = False
> +    inlist = False
> +    edit_count = 0
> +    for lineno, line in enumerate(contents):
> +        code, comments = split_code_comments(line)

Nitpicking, but maybe instead of calling strip() all over the place, 
just count the number of spaces and strip right at the outset? E.g. 
something like:

stripped = code.strip()
line_indent = len(code) - len(stripped)

You can then reason about indent levels by comparing stripped to code 
afterwards, and avoid doing this:

> +            # skip further subarrays or lists
> +            if '[' in code or ']' in code:
> +                continue
> +            if not code.startswith(indent) or code[len(indent)] == ' ':

Opting to just check the indent size you calculated initially. Unless 
i'm missing something :)

You could also increment edit_count if `calculated indent + stripped` is 
equal to `code`. Seems easier logic than raw string manipulation you're 
going for here...

<snip>

> +def process_file(filename, fix):
> +    '''run checks on file "filename"'''
> +    if VERBOSE:
> +        print(f'Processing {filename}')
> +    with open(filename) as f:
> +        contents = [ln.rstrip() for ln in f.readlines()]

So any trailing whitespace gets automatically and silently fixed?

> +
> +    if check_indentation(filename, contents) > 0 and fix:
> +        print(f"Fixing {filename}")
> +        with open(filename, 'w') as f:
> +            f.writelines([f'{ln}\n' for ln in contents])

Something seems suspect here. So, if `fix` is *not* specified, the 
script just opens the file, reads it, and... does nothing else?

-- 
Thanks,
Anatoly