Re: [PATCH 6/9] dtc: dt-check-style: Fix alignment of values in continued property lines

Krzysztof Kozlowski <[email protected]> Sun, 2 Aug 2026 10:22:21 +0200
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
On 26/07/2026 17:56, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The new alignment rule incorrectly forces a standalone closing bracket (e.g., '>;') to align with inner values instead of the opening bracket.
> - [Low] The hardcoded error message is misleading when target_offset == 1, as it claims the expected column is 'under < or "'.
> --
> 
> --- Patch [6]: [PATCH 6/9] dtc: dt-check-style: Fix alignment of values in continued property lines ---
> commit 3d17d86c35121d71a779318164c4a25fa384d8c1
> Author: Krzysztof Kozlowski <[email protected]>
> 
> dtc: dt-check-style: Fix alignment of values in continued property lines
> 
> This commit updates the dt-check-style script to enforce that continued
> property lines aligning to inner values are indented one space past the
> opening '<' or '"'. It also includes tests to verify this behavior.
> 
>> diff --git a/scripts/dtc/dt-check-style b/scripts/dtc/dt-check-style
>> index 4d90fbec0e391..171a5e12fcbd8 100755
>> --- a/scripts/dtc/dt-check-style
>> +++ b/scripts/dtc/dt-check-style
> 
> [ ... ]
> 
>> @@ -928,15 +929,18 @@ def check_continuation_alignment(ctx):
>>              continue
>>          # First '<' or '"' after '='
>>          rest = dl.raw[eq + 1:]
>> -        m = re.search(r'[<"]', rest)
>> +        m = re.search(r'\s*([<"])', rest)
>>          if not m:
>>              continue
>> -        target_col = _display_col(dl.raw[:eq + 1 + m.start()])
>> +        target_col = _display_col(dl.raw[:eq + 1 + m.start(1)])
>>          for cont in dl.continuations:
>> -            if _display_col(cont.indent_str) != target_col:
>> +            target_offset = 0
>> +            if not re.match(r'[<"]', cont.stripped):
>> +                target_offset = 1
> 
> [Severity: Medium]
> Will this new alignment rule incorrectly flag standalone closing brackets?
> 
> If a continuation line is just a closing bracket like ">;", the script
> evaluates re.match(r'[<"]', cont.stripped) as false, which sets
> target_offset = 1. This forces developers to indent the closing bracket
> one space to the right of the opening bracket instead of aligning them.

There is no rule for that yet.

> 
>> +            if _display_col(cont.indent_str) != target_col + target_offset:
>>                  yield (cont.lineno,
>>                         'continuation should align to column %d '
>> -                       '(under < or ")' % (target_col + 1))
>> +                       '(under < or ")' % (target_col + target_offset + 1))
> 
> [Severity: Low]
> Is this error message accurate when target_offset is 1?
> 
> When the script expects alignment to the inner value (target_offset == 1),
> the error message still tells developers to align "(under < or ")". This
> could be confusing since the target column provided is actually one space
> to the right of the bracket or quote.

I'll improve err msg.


Best regards,
Krzysztof