Add --init flag to handle missing frontmatter

[nhomble]

Summary

• When --init is passed, treat missing frontmatter as empty {}
• Allows initializing frontmatter on files that don't have it

Examples

# Initialize frontmatter on a file without it
echo 'Just body content' | fmq --init '.title = "Hello"'
# ---
# title: Hello
# ---
# Just body content

# Query missing frontmatter (returns null/empty)
echo 'No frontmatter' | fmq --init '.title'
# null

Test plan

• cargo test passes
• Manual testing with --init flag

[claude]

Pull Request Review

Thank you for this well-thought-out addition! The --init flag is a useful feature that enables initializing frontmatter on files that don't have it. Here's my detailed review:

✅ Strengths

1. Clear Feature Scope: The PR addresses a specific use case (initializing frontmatter on files without it) with a focused implementation
2. Consistent API Design: The allow_empty parameter follows the existing pattern of the need_body parameter in extract_reader
3. Good Documentation: The PR description includes clear examples of the new functionality
4. Manual Testing: The author verified the functionality works as expected

🐛 Potential Issues

1. Inconsistent Behavior Between extract and extract_reader (Medium Priority)

The extract function (line 21 in frontmatter.rs) doesn't support the allow_empty parameter, which creates an API inconsistency:

• extract_reader can now handle missing frontmatter when allow_empty=true
• extract always fails on missing frontmatter
• The fmq function (lib.rs:40) uses extract, so it doesn't benefit from the --init flag

Impact: Users can only use --init via the CLI when reading from files/stdin, not when using the library directly with string inputs.

Recommendation: Consider adding an allow_empty parameter to the extract function as well, or document this limitation clearly.

2. Empty String vs Missing Frontmatter Semantics (Low Priority)

In lib.rs:56-60, empty frontmatter is converted to "{}":

let frontmatter = if doc.frontmatter.is_empty() {
    "{}".to_string()
} else {
    doc.frontmatter.clone()
};

This is correct for the use case, but worth noting that a file with actual empty frontmatter (---\n---\n) would be treated differently from a file with no frontmatter at all when using the string-based API.

Current behavior:

• No frontmatter + --init → treats as {}
• Empty frontmatter (---\n---\n) → also empty string → also treats as {}

This is probably fine, but documenting this behavior would be helpful.

🔒 Security Considerations

Status: ✅ No security concerns identified

• No new attack vectors introduced
• Proper error handling for I/O operations
• No unsafe code or external command execution

⚡ Performance Considerations

Status: ✅ Minimal performance impact

• The additional logic only executes when frontmatter is missing
• Early return pattern in frontmatter.rs:61-70 is efficient
• String allocation in lib.rs:57 only happens when needed

Minor optimization opportunity: The clone() on line 59 of lib.rs could be avoided by borrowing, but this is not significant since the frontmatter string is typically small.

🧪 Test Coverage

Concerns: ⚠️ No automated tests for the new functionality

While the author confirmed manual testing, the PR lacks:

1. Unit tests for extract_reader with allow_empty=true

   • Test case: file with no frontmatter
   • Test case: empty file
   • Test case: file with only body content

2. Integration tests for the --init flag

   • No fixture-based tests in tests/fixtures/ directory
   • The existing integration.rs test suite structure supports this

3. Tests for the fmq_reader empty frontmatter handling

   • Verify that empty frontmatter is treated as {}
   • Test mutations with --init flag

Recommendation: Add test coverage following the existing pattern. Example fixture structure:

tests/fixtures/init/
  ├── no-frontmatter/
  │   ├── input.md (no frontmatter)
  │   ├── expr.txt (.title = "Hello")
  │   └── output.md (expected result)

📝 Code Quality

Status: ✅ Good overall quality

• Follows existing code style and patterns
• Proper error handling
• Clear variable names
• Documentation in PR description is helpful

Minor suggestion: Consider adding a doc comment to the allow_empty parameter explaining when it should be used:

/// If true, treat missing frontmatter as an empty document rather than returning an error.
/// This enables initializing frontmatter on files that don't have it.
allow_empty: bool,

🎯 Recommendations

Must Fix:

• None (the code is functional as-is)

Should Fix:

1. Add automated tests for the new functionality (following existing test patterns)
2. Consider adding allow_empty parameter to extract() function for API consistency

Nice to Have:

1. Add doc comments to the new parameter
2. Update README.md with --init flag documentation and examples
3. Consider adding a changelog entry if the project maintains one

Summary

This is a solid, focused PR that adds useful functionality. The implementation is clean and follows existing patterns. The main gap is test coverage - adding automated tests would make this production-ready. The API inconsistency between extract and extract_reader is worth considering, but not a blocker.

Overall Assessment: ✅ Approve with recommendation to add tests

---

🤖 Generated with Claude Code

[claude]

Code Review

Thanks for this PR! The --init flag is a useful addition that allows handling files without frontmatter. Overall, the implementation is solid and well-tested. Here are my findings:

✅ Strengths

1. Good test coverage: You've added comprehensive e2e tests covering both query and mutation scenarios with the --init flag
2. Consistent with existing patterns: The implementation follows the established fixture-based testing approach
3. Clear PR description: Good examples showing the feature in action
4. Minimal API surface: The flag is appropriately scoped to the CLI interface

🔍 Code Quality Observations

src/frontmatter.rs:50-73
The allow_empty parameter handling looks good. The early return when frontmatter is missing preserves the body content correctly.

src/lib.rs:52-70
Clean implementation with good separation of concerns. The empty frontmatter defaulting to "{}" is reasonable for jq compatibility.

🐛 Potential Issues

Minor: Code duplication in lib.rs
The frontmatter variable handling (lines 56-60) could be slightly simplified:

let frontmatter = if doc.frontmatter.is_empty() {
    "{}"
} else {
    &doc.frontmatter
};

let result = run(expr, frontmatter)?;

This avoids the unnecessary clone() and allocation of a new String when frontmatter is not empty.

Edge case consideration:
What happens when a user runs --init on a file that does have frontmatter? Currently it would work fine, but the flag name suggests it's only for initialization. Consider if this is the desired behavior or if there should be validation.

🚀 Performance Considerations

• The additional clone() on line 59 in src/lib.rs creates an unnecessary allocation in most cases (see suggestion above)
• The allow_empty parameter adds minimal overhead and only affects the error path

🔒 Security Concerns

No security issues identified. The changes don't introduce any unsafe operations or external input handling vulnerabilities.

📝 Test Coverage

Excellent! You've covered:

• ✅ Mutation with --init (add-title)
• ✅ Query with --init (query-missing)

Suggestions for additional test coverage:

1. Test --init flag behavior on a file that already has frontmatter (should work normally)
2. Test mutation operations that delete/modify in --init mode
3. Consider unit tests for the extract_reader function with allow_empty=true in src/frontmatter.rs

💡 Suggestions

1. Documentation: Consider adding the --init flag to the README.md with examples
2. Error messages: When using --init, error messages about "no frontmatter found" won't appear, which is good. The UX is clear.
3. API consistency: The fmq function doesn't support the init behavior, only fmq_reader does. This is fine if intentional, but worth documenting.

📊 Summary

Approve with minor suggestions

This is a well-implemented feature that addresses a real use case. The code is clean, well-tested, and follows existing conventions. The only recommendation is the minor optimization to avoid unnecessary string cloning.

Great work! 🎉