fix: replace broken "./" output default with a derived output path#4
fix: replace broken "./" output default with a derived output path#4pnodet wants to merge 1 commit into
Conversation
The default output value "./" is a directory, so File::create always failed and every run without -o errored. Default now derives <input>.cleaned.las next to the input file, and paths are PathBuf instead of String. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Thermo-nuclear code quality reviewScope: the CLI/path-boundary change only (streaming, dropped-point reporting, and uuid removal are owned by sibling PRs and were not re-judged here). Verdict: approve. This is the right shape of fix, not a patch-over. The old I verified
All sensible, and structurally the derived path can never equal the input: Two minor nits, neither blocking:
|
Bug
Args.outputdefaulted to"./", which is a directory.las::Writer::from_path("./")callsFile::create("./"), which always fails with "Is a directory" — so every invocation without-oerrored out. The default could never work.Fix
outputis nowOption<PathBuf>with no default value. When omitted, the output path is derived from the input: same directory, file stem plus a.cleaned.lasextension (e.g.-p /data/scan.laswrites/data/scan.cleaned.las).pathis nowPathBufinstead ofString, and theReader::from_path/Writer::from_pathcall sites pass references to the new types.--outputhelp text documents the default behavior.Verified with
cargo buildand a check thatwith_extension("cleaned.las")onscan.lasyieldsscan.cleaned.las.🤖 Generated with Claude Code