Actionable comments posted: 1

cpplint.py (1)

4164-4170: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Delay the multiline template check for better performance.

Currently, IsMultilineFunctionTemplateDeclaration runs for every line in the file. This results in unnecessary backward string scanning and regex evaluations for the vast majority of lines (e.g., inside function bodies or outside namespaces).

Moving this check inside the ShouldCheckNamespaceIndentation block ensures we only incur this cost for lines that are actually candidates for the namespace indentation warning.

⚡ Proposed refactor

-    if IsMultilineFunctionTemplateDeclaration(clean_lines, line):
-        return
-
     if ShouldCheckNamespaceIndentation(
         nesting_state, is_namespace_indent_item, clean_lines.elided, line
     ):
+        if IsMultilineFunctionTemplateDeclaration(clean_lines, line):
+            return
         CheckItemIndentationInNamespace(filename, clean_lines.elided, line, error)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint.py` around lines 4164 - 4170, Move the
IsMultilineFunctionTemplateDeclaration check inside the
ShouldCheckNamespaceIndentation conditional, after confirming the line is a
namespace-indentation candidate and before calling
CheckItemIndentationInNamespace; remove the unconditional early return so the
expensive scan is skipped for unrelated lines.

cpplint_unittest.py (1)

326-357: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test case for function template definitions.

The tests added here only cover function template declarations. To ensure that function template definitions (which include a { block) are also correctly exempted from false positive warnings and don't regress in the future, consider adding a test case for them. This will also verify the bug fix proposed in cpplint.py.

🧪 Proposed test addition

         ]
         assert self.GetNamespaceResults(lines) == ""
 
+        lines = [
+            "namespace Test {",
+            "template <typename Type1,",
+            "          typename Type2>",
+            "void TestFunc(const Type1 &var1, Type2 &var2) {",
+            "}",
+            "}  // namespace Test",
+        ]
+        assert self.GetNamespaceResults(lines) == ""
+
     def testNamespaceIndentationIndentedMultilineFunctionTemplateDeclaration(self):

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint_unittest.py` around lines 326 - 357, Add coverage in the namespace
indentation tests for multiline function template definitions, including a body
block with `{`, and verify correctly aligned definitions produce no warnings
while indented definitions report the expected namespace-indentation violations.
Extend the existing test methods near
testNamespaceIndentationMultilineFunctionTemplateDeclaration, reusing
GetNamespaceResults and the established declaration expectations.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpplint.py`:
- Around line 7408-7415: Update the declaration handling around the existing
function-template detection to isolate the signature by splitting on either “;”
or “{” before checking for braces and matching the function name. Preserve the
existing exemptions for empty declarations, class/enum/struct/using
declarations, and assignment operators while allowing function template
definitions to return the same result as declarations.

---

Nitpick comments:
In `@cpplint_unittest.py`:
- Around line 326-357: Add coverage in the namespace indentation tests for
multiline function template definitions, including a body block with `{`, and
verify correctly aligned definitions produce no warnings while indented
definitions report the expected namespace-indentation violations. Extend the
existing test methods near
testNamespaceIndentationMultilineFunctionTemplateDeclaration, reusing
GetNamespaceResults and the established declaration expectations.

In `@cpplint.py`:
- Around line 4164-4170: Move the IsMultilineFunctionTemplateDeclaration check
inside the ShouldCheckNamespaceIndentation conditional, after confirming the
line is a namespace-indentation candidate and before calling
CheckItemIndentationInNamespace; remove the unconditional early return so the
expensive scan is skipped for unrelated lines.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 484524ee-ff9f-4aef-8dc6-575e23645c1c

Reviewing files that changed from the base of the PR and between 6be492a and cd67aa7.

• CHANGELOG.rst
• cpplint.py
• cpplint_unittest.py
• samples/boost-sample/simple.def

I have reviewed it locally: "pytest cpplint_unittest.py -k NamespaceIndentation" passes
(6 passed), and a manual repro confirms the behaviour, a
"template <...>\nvoid F(...)" continuation at column 0 no longer triggers
"whitespace/indent_namespace", while the same template indented is still
flagged twice (matches the negative test). The boost-sample delta
(2900->2898, octonion.hpp:675-676) lines up with the intended removal.

The fix is well-targeted: IsMultilineFunctionTemplateDeclaration only
suppresses lines that fall within a multiline "template<...>" span whose
post-">" declaration resolves to a function, and it correctly bails for
class/struct/enum/using so template type declarations stay checked.

One non-blocking suggestion: the class/struct/enum/using exclusion branch
isn't directly covered — adding a case (e.g. a multiline
"template<...>\n class Foo;" indented) asserting it's still flagged would
lock that distinction in against future regressions.

In general, I'm not sure why we should bail if it's not a function. Was the F+ triggerable with template usage in non-functions too?

Even within the function-declaration scope of this change, the template-span scan still misses valid continuation lines when an earlier template parameter contains braces or a semicolon. For example:

namespace Test {
template <typename T,
          bool Valid = requires { typename T::value_type; },
          typename U = void>
void Func();
}  // namespace Test

On cd67aa7, the typename U = void line still emits whitespace/indent_namespace. IsMultilineFunctionTemplateDeclaration() walks backward and returns at [;{}] before finding the enclosing template <, even though those tokens belong to the requires-expression inside the template parameter list.

Please identify the enclosing template-parameter span before treating statement delimiters as boundaries, and add a regression with a later continuation line after a requires-expression or braced default argument. The full suite passes (227 passed), so this boundary currently has no focused coverage.