Skip to content

docs: fix (T, bool) return pattern documentation#51

Merged
crhntr merged 1 commit intomainfrom
docs/fix-bool-return-behavior
Dec 6, 2025
Merged

docs: fix (T, bool) return pattern documentation#51
crhntr merged 1 commit intomainfrom
docs/fix-bool-return-behavior

Conversation

@crhntr
Copy link
Copy Markdown
Member

@crhntr crhntr commented Dec 6, 2025

The documentation incorrectly stated that bool=true skips template execution. The actual behavior is bool=false skips template (you already handled it), bool=true continues to template execution.

The documentation incorrectly stated that bool=true skips template execution.
The actual behavior is bool=false skips template (you already handled it),
bool=true continues to template execution.
@claude
Copy link
Copy Markdown

claude Bot commented Dec 6, 2025

Code Review - PR #51: Fix (T, bool) Return Pattern Documentation

Summary

APPROVED - This is an important documentation fix that corrects a critical inversion in the boolean return value semantics.

Changes Reviewed

The PR corrects documentation in docs/reference/call-results.md about the (T, bool) return pattern. The old documentation incorrectly stated that bool=true skips template execution, when the actual behavior is the opposite.

Verification

I verified this fix against the actual implementation in internal/generate/routes.go:1215-1236:

if basic, ok := lastResult.(*types.Basic); ok && basic.Kind() == types.Bool {
    return []ast.Stmt{
        // ...
        &ast.AssignStmt{Lhs: []ast.Expr{dataVar, ast.NewIdent(okIdent)}, Tok: token.ASSIGN, Rhs: []ast.Expr{call}},
        &ast.IfStmt{
            Cond: &ast.UnaryExpr{Op: token.NOT, X: ast.NewIdent(okIdent)},  // if !ok
            Body: &ast.BlockStmt{
                List: []ast.Stmt{
                    &ast.ReturnStmt{},  // return early, skip template
                },
            },
        },
        // ... continues to set result and okay=true if ok was true
    }, nil
}

The logic clearly shows:

  • bool=false → Handler returns immediately (skip template execution) ✅
  • bool=true → Continues to template execution ✅

This is also confirmed by the test in cmd/muxt/testdata/reference_call_with_bool_return.txt:

  • When ReturnOK: false, response body is empty (template skipped)
  • When ReturnOK: true, template executes and renders "Ok: true"

Code Quality Assessment

✅ Correctness

  • All documentation changes are accurate and match the implementation
  • The semantic description is now clearer: "I handled it, skip template" vs "Continue to template execution"

✅ Consistency

  • Updated all occurrences of the pattern across the documentation
  • Table entries, code examples, and behavior description are all aligned

✅ Clarity

The new wording is more intuitive:

  • bool=false → "I handled it, skip template" (you already wrote the response)
  • bool=true → "Continue to template execution" (proceed normally)

Impact

This is a documentation-only change with no code modifications, so there are no:

  • Performance concerns
  • Security implications
  • Test coverage changes needed

Additional Notes

The corrected documentation aligns well with the common Go pattern where a boolean return value indicates "success" or "should continue", making true mean "continue" and false mean "already handled/stop".

Recommendation

Approve and merge. This fix corrects important user-facing documentation that could lead to significant confusion and bugs if left incorrect.


🤖 Generated with Claude Code

@crhntr crhntr merged commit 12e4276 into main Dec 6, 2025
2 checks passed
@crhntr crhntr deleted the docs/fix-bool-return-behavior branch December 6, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant