Add --inplace/-i flag to format files in place

[gmr]

Summary

• Adds --inplace / -i CLI flag that writes formatted output back to the source file instead of stdout
• Uses clap conflicts_with to enforce mutual exclusivity with --check
• Validates that at least one file is provided when using --inplace

Test plan

• pgfmt -i file.sql formats and overwrites file.sql
• pgfmt --inplace file1.sql file2.sql formats multiple files in place
• pgfmt -i --check file.sql errors with clap conflict message
• echo "SELECT 1" | pgfmt -i errors requiring files
• pgfmt file.sql still prints to stdout (default behavior unchanged)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added --inplace / -i flag to write formatted SQL back to input files (conflicts with --check).

• Behavior Changes

  • --check continues to only report "Would reformat" without writing.
  • --inplace restores original file permissions after writing, errors if used with stdin, and is rejected when no file paths are provided.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1e666a8e-026f-4c3b-940e-bdeadbf521cc

📥 Commits

Reviewing files that changed from the base of the PR and between 6b01632 and 304112a.

📒 Files selected for processing (1)

• src/main.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/main.rs

---

📝 Walkthrough

Walkthrough

Added an --inplace/-i CLI flag (conflicts with --check) and extended process to accept an optional PathBuf. With --inplace and files provided, formatted SQL is written back to each file (preserving permissions). --inplace is rejected for stdin or when no file paths are given.

Changes

Cohort / File(s)	Summary
In-place File Modification Feature src/main.rs	Added --inplace/-i CLI flag with conflicts_with = "check". Changed process signature to accept path: Option<&PathBuf> and write formatted SQL back to files when --inplace is set. Added validation to reject --inplace for stdin or when no file paths are provided.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as "CLI Parser"
    participant Run as "run() / Validator"
    participant Process as "process(name, sql, path, cli)"
    participant FS as "File I/O / Permissions"
    participant Stdout as "Stdout / Reporter"

    User->>CLI: invoke with flags and args
    CLI->>Run: parsed args (incl. --inplace / --check)
    Run->>Run: validate conflicts & presence of file paths
    alt invalid (no files or stdin with --inplace)
        Run->>User: return error
    else valid with file paths and --inplace
        Run->>Process: call with path(s)
        Process->>Process: format SQL
        alt check mode
            Process->>Stdout: report "Would reformat"
        else inplace mode
            Process->>FS: write formatted SQL, restore perms
            FS->>Stdout: confirm update
        end
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I hop in code to change a line,
A flag to write the files just fine.
Permissions kept, no stdin spree,
Conflict checks keep things tidy, see?
A tiny tweak — formatted and free.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change—adding an --inplace/-i flag for in-place file formatting. It is concise, specific, and clearly captures the primary objective of the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/inplace-flag

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main.rs (1)

42-44: Consider preserving file permissions when writing in place.

fs::write may alter the file's permissions on some platforms. If the original file had non-default permissions (e.g., 0o600), they could be reset.

♻️ Suggested approach to preserve permissions

+use std::os::unix::fs::PermissionsExt;
+
 fn process(name: &str, sql: &str, path: Option<&PathBuf>, cli: &Cli) -> Result<bool, String> {
     // ...
     } else if cli.inplace {
         let path = path.ok_or_else(|| "cannot use --inplace with stdin".to_string())?;
+        let metadata = fs::metadata(path).map_err(|e| format!("{name}: {e}"))?;
+        let permissions = metadata.permissions();
         fs::write(path, &formatted).map_err(|e| format!("{name}: {e}"))?;
+        fs::set_permissions(path, permissions).map_err(|e| format!("{name}: {e}"))?;
     }

Alternatively, for maximum safety (atomic writes), write to a temp file in the same directory and rename it over the original. This prevents corruption if the process is interrupted mid-write. This is optional for a CLI tool of this nature.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` around lines 42 - 44, The inplace write uses fs::write which can
change file permissions; modify the cli.inplace branch (where path, formatted
are used) to preserve permissions by reading the original file metadata
(fs::metadata) and capturing its permissions, then write the output to a
temporary file in the same directory (e.g., using a unique temp name next to
path), set the temp file's permissions to the original permissions
(fs::set_permissions or OpenOptions with mode on Unix), and atomically rename
the temp file to the original path (fs::rename); ensure errors are propagated
the same way as existing map_err formatting.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/main.rs`:
- Around line 42-44: The inplace write uses fs::write which can change file
permissions; modify the cli.inplace branch (where path, formatted are used) to
preserve permissions by reading the original file metadata (fs::metadata) and
capturing its permissions, then write the output to a temporary file in the same
directory (e.g., using a unique temp name next to path), set the temp file's
permissions to the original permissions (fs::set_permissions or OpenOptions with
mode on Unix), and atomically rename the temp file to the original path
(fs::rename); ensure errors are propagated the same way as existing map_err
formatting.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cc051b56-a0f8-452e-9d85-f71447fbecb8

📥 Commits

Reviewing files that changed from the base of the PR and between 2f576ed and 6b01632.

📒 Files selected for processing (1)

• src/main.rs