Add --in-place flag for editing files directly

[nhomble]

Adds -i/--in-place flag that reads the file, applies the mutation, and writes the result back to the same file. This is useful for scripting workflows where you want to update frontmatter in place.

• Updated extract() to accept allow_empty parameter for init support
• Updated fmq() to accept init parameter
• Added CLI integration test that verifies file is modified correctly

[claude]

Pull Request Review: Add --in-place flag for editing files directly

Summary

This PR adds the -i/--in-place flag to allow editing markdown files directly, similar to sed's in-place editing. The implementation is solid overall with good test coverage.

Code Quality & Best Practices ✅

Strengths:

• Clean separation of concerns between CLI and library logic
• Consistent error handling patterns throughout
• Good use of Rust idioms (pattern matching, unwrap_or_else)
• Backwards compatibility maintained - existing API extended properly

API Design:
The allow_empty/init parameter threading is well-designed:

• extract() in src/frontmatter.rs:21 now accepts allow_empty: bool
• fmq() in src/lib.rs:40 now accepts init: bool
• fmq_reader() in src/lib.rs:59 also updated consistently

Potential Issues & Suggestions

1. Data Loss Risk - No Backup ⚠️

The in-place editing at src/main.rs:48 directly overwrites files without creating backups. If the mutation fails partway through or produces unexpected results, the original file is lost.

Recommendation: Consider one of these approaches:

• Create a .bak backup file before writing (like sed -i.bak)
• Write to a temp file first, then atomic rename (safer)
• Add a --backup flag for user control

Example atomic write pattern:

let temp_file = path.with_extension("tmp");
fs::write(&temp_file, output)?;
fs::rename(&temp_file, path)?;

2. Empty Frontmatter Handling 🤔

In src/lib.rs:43-47, when doc.frontmatter.is_empty(), the code defaults to "{}". This is reasonable but:

• It's duplicated in both fmq() and fmq_reader()
• The behavior isn't documented

Suggestion: Extract this to a helper function or add a comment explaining why empty frontmatter becomes "{}".

3. Test Coverage - Edge Cases

The integration test at tests/integration.rs:133 is excellent, but missing some edge cases:

Uncovered scenarios:

• What happens with --in-place + --init on a file without frontmatter?
• What if the file is read-only or doesn't exist?
• What if the expression is a query (not a mutation) with --in-place?

Recommendation: Add test cases for:

#[test]
fn in_place_with_init() { /* ... */ }

#[test]
fn in_place_query_is_noop() { /* ... */ }

#[test]
fn in_place_readonly_file() { /* ... */ }

4. Minor: Inconsistent Naming 📝

The parameter is called allow_empty in extract() but init everywhere else. While this makes sense semantically (allow_empty is implementation detail, init is user-facing), consider using init consistently or documenting why they differ.

Performance Considerations

Current approach (src/main.rs:38-48):

1. Read entire file into memory
2. Process it
3. Write back

This is fine for markdown files (typically small), but:

• For very large files, memory usage could be a concern
• The non-in-place path uses BufReader for streaming

Assessment: Not a real issue for the intended use case (markdown frontmatter), but worth noting.

Security Concerns

File Race Condition ⚠️

Between reading at src/main.rs:38 and writing at src/main.rs:48, another process could modify the file. This is a classic TOCTOU (Time-of-check to time-of-use) issue.

Impact: Low - markdown files are rarely edited concurrently
Mitigation: Document this limitation or use file locking if needed

Path Traversal ✅

No issues - PathBuf handles paths safely, and the tool doesn't construct paths from user input.

Testing

Test Coverage: Excellent ✅

• Integration test verifies actual file modification
• Uses existing mutation fixtures for comprehensive coverage
• Tests cleanup properly (tests/integration.rs:179)
• Uses env!("CARGO_BIN_EXE_fmq") correctly for binary path

Test Quality:

• Clear assertions with helpful error messages
• Proper use of temp directories
• Could benefit from additional edge case coverage (see above)

Documentation

Missing:

• README.md doesn't show --in-place usage examples
• No mention of the combination of --init and --in-place

Suggested README addition:

# Edit file in place
fmq '.title = "New Title"' post.md --in-place

# Initialize frontmatter if missing
fmq '.title = "Hello"' new-post.md --init --in-place

Overall Assessment

Score: 8/10

This is a well-implemented feature that follows the existing codebase patterns. The main concerns are:

1. Lack of backup mechanism (data loss risk)
2. Missing edge case tests
3. Missing documentation updates

Recommendation:

• Merge with minor changes - The data loss risk should be addressed (either via backup or clear documentation warning)
• Add the suggested test cases
• Update README with --in-place examples

Great work on maintaining consistency with the existing codebase and providing solid test coverage! 🎉