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