nmoroze/tclint

Fix indentation check for continuations of commands starting on same line as script

Opened this issue · 5 comments

nmoroze commented

Examples:

if {1} {puts $chan \
    "foo"}

Actual:

test.tcl:2:1: expected indent of 0 spaces, got 4 [indent]

Expected: no violations

eval cmd \
a \
    b

Actual:

test.tcl:3:1: expected indent of 0 spaces, got 4 [indent]

Expected:

test.tcl:2:1: expected indent of 4 spaces, got 0 [indent]
nmoroze commented

@maliberty thanks for the pointer, I'll take a look ASAP.

In the meantime, you can add comments to locally disable indent checking and get your CI back to green.

# tclint-disable indent
[ code ]
# tclint-enable indent

See docs for more info.

BTW, I think tclint will want a different style than you have, e.g. it'll ask for:

mpl2::set_debug_cmd $block \
  $coarse \
  $fine \
  ...

If you prefer the aligned style I can look into supporting it, but I think that'll be lower priority.

maliberty commented

We have lots of other code like this that doesn't get flagged. Any idea why just here?

nmoroze commented

The violation on line 313 is related to the # checker comment. Dropping the closing brace to a lower line seems to be how this is done in other parts of the codebase, e.g. https://github.com/The-OpenROAD-Project/OpenROAD/blob/efe05a9c9aee2955753945eca8a9e7aaee40b8cd/src/pad/src/pad.tcl#L142. That fixes the problem, although this is a bug in tclint that I'll address (#48).

I think the remaining violations are actually just tclint working as intended. Do you have any examples of it allowing that style elsewhere? There are two exceptions where it won't check the indentation of multiline commands (in expressions [#20] and command substitutions [#23]), so it's possible the other examples you've seen have been one of those two cases (e.g. the mpl2::rtl_macro_placer_cmd in mpl.tcl is inside an if statement expression, so it's not currently being checked).

maliberty commented

Perhaps it is just that those two exceptions dominate. I would like to see the indented style supported. For now I've used your disable suggestion.