Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1518 +/- ##
==========================================
- Coverage 91.98% 91.97% -0.01%
==========================================
Files 88 88
Lines 18374 18390 +16
==========================================
+ Hits 16901 16914 +13
- Misses 1473 1476 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📦 Cargo Bloat ComparisonBinary size change: +0.44% (22.5 MiB → 22.6 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
There was a problem hiding this comment.
Pull request overview
This PR improves the calculation of maximum CLI argument lengths by adopting a more accurate approach from the argmax library. The changes primarily focus on better accounting for pointer sizes, environment variables, and platform-specific limitations.
Changes:
- Refactored CLI argument length calculation to use more precise size accounting with pointer overhead and null terminators
- Extracted static
ARG_MAXandPAGE_SIZEvalues usinglibc::sysconffor Unix systems - Updated
Partitionsiterator to track remaining argument length instead of computing current length - Made an unrelated syntax fix in the Haskell language module
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| crates/prek/src/run.rs | Core refactoring of CLI argument length calculation with new helper functions and improved platform-specific handling |
| crates/prek/src/languages/haskell.rs | Unrelated syntax fix changing closure parameter order in get_or_try_init call |
Comments suppressed due to low confidence (1)
crates/prek/src/run.rs:220
- The panic message uses the old calculation method
filename.as_os_str().len() + 1instead of the newarg_size()function. This creates an inconsistency where:
- The check uses
arg_size(filename)to determine if a file fits (line 206) - The panic message reports the size using a different calculation (line 220)
This inconsistency could confuse users because the reported size won't match what the code actually checked. The panic message should use arg_size(filename) for consistency and accuracy.
// is too long to fit in the command line by itself.
Adapted from https://github.com/sharkdp/argmax