refactor: adopt Rust 1.95 cfg_select! macro at platform-conditional sites#3846
Merged
Conversation
…ites Convert 15 sites across 9 files from paired #[cfg(...)] let-bindings and duplicate-fn definitions to std's new cfg_select! macro, which reads like a compile-time match. Notable wins: reset_sigpipe and is_executable collapse from 2 fn defs to 1; the platform_factor 4-arm target_os ladder becomes a single expression; the 9 target_endian simd_json/serde_json pairs lose their #[cfg] scaffolding. The stats.rs `mut stat_args` site (cache-args deserialization) is left on #[cfg] — its little-endian arm consumes the String into a Vec<u8> while the big-endian arm borrows it, and cfg_select!'s expression-form arms can't carry multi-statement blocks without restructuring ownership. Net -23 lines; cargo build/clippy clean; 187 tests across affected commands pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors platform-conditional code paths to use Rust 1.95’s cfg_select! macro, reducing duplicated #[cfg(...)] scaffolding and consolidating platform/endian-specific logic into single expressions/items.
Changes:
- Replaced paired
#[cfg]blocks withcfg_select!for OS-/endian-specific JSON (de)serialization and other platform conditionals. - Collapsed duplicated platform-specific implementations (e.g.,
reset_sigpipe,is_executable) into unified definitions. - Simplified a few platform-dependent constants/bindings (e.g.,
platform_factor, Windows row-count fallback,whichvswhere).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/util.rs | Consolidates unix/windows + endian-specific branches using cfg_select! (signals, executability, JSON paths, platform factors). |
| src/cmd/validate.rs | Uses cfg_select! for endian-specific JSON schema deserialization. |
| src/cmd/tojsonl.rs | Uses cfg_select! to select Windows row-count fallback implementation. |
| src/cmd/stats.rs | Converts multiple endian-specific JSON serialization/deserialization sites to cfg_select!. |
| src/cmd/scoresql.rs | Selects which vs where via cfg_select! for cross-platform PATH lookup. |
| src/cmd/schema.rs | Uses cfg_select! for endian-specific JSON pretty-print serialization. |
| src/cmd/prompt.rs | Uses cfg_select! to apply macOS-only dialog configuration without duplicating builder setup. |
| src/cmd/pragmastat.rs | Uses cfg_select! for endian-specific cache record parsing. |
| src/cmd/foreach.rs | Uses cfg_select! for unix/windows program path handling and dry-run string formatting. |
| src/cmd/apply.rs | Refactors the thousands-formatting conditional branch structure. |
… flip The clippy::if_not_else cleanup in e2a17bc flipped the branches but left the explanatory comment describing the old `!= 0.0` flow. Rewrite it to match the new condition (the `> 0.0` rationale about preserving negative fractions is still relevant). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#[cfg(...)]attributes (or duplicated function defs) to std's newcfg_select!macro stabilized in Rust 1.95 (current MSRV).reset_sigpipeandis_executablecollapse from 2 fn defs to 1; theplatform_factor4-armtarget_osladder becomes a single expression; the 9target_endiansimd_json/serde_json pairs lose their#[cfg]scaffolding.Sites converted
src/util.rsreset_sigpipe— merged unix / not(unix) defsis_executable— merged unix / windows defsplatform_factor— 4-armtarget_osmatchtarget_endiansimd_json/serde_json sitesreturn Errinto singlecfg_select!over the messagesrc/cmd/stats.rs— 3target_endiansites (incl.sniff_resultwithletpulled outside)schema.rs,pragmastat.rs,validate.rs— 1target_endiansite eachforeach.rs— 2 unix/windowsletbindingstojsonl.rs—count_rows_regularvscount_rowsscoresql.rs—whichvswhereprompt.rs— macOS-onlyset_can_create_directoriesOne site intentionally kept on
#[cfg]stats.rs:1398(mut stat_argsfor stats-args cache deserialization). The little-endian arm consumes theStringinto aVec<u8>while the big-endian arm borrows it.cfg_select!'s expression-form arms must be single expressions, so squeezing this through required restructuring ownership without a clear win.Notes on
cfg_select!In expression position, brace-block arms must contain a single expression — multi-statement
{ let x = ...; expr }blocks trigger "trailing semicolon in macro" errors. Workaround forsniff_result(stats.rs) andvalidate.rs: pull thelet mutoutsidecfg_select!(using#[cfg_attr(target_endian = "big", allow(unused_mut))]to suppress the cross-arch warning). Item-position usage works fine with multi-statement blocks (e.g.,is_executable).Test plan
cargo build --locked --bin qsv -F all_features— cleancargo build --bin qsvlite -F lite— clean (only pre-existingdescribegpt.rswarning)cargo build --bin qsvmcp -F qsvmcp— cleancargo clippy --bin qsv -F all_features— cleancargo test --test tests stats_cache -F all_features— 23 passedcargo test --test tests test_validate -F all_features— 62 passedcargo test --test tests test_schema -F all_features— 12 passedcargo test --test tests test_tojsonl -F all_features— 25 passedcargo test --test tests test_foreach -F all_features— 18 passedcargo test --test tests test_pragmastat -F all_features— 47 passed🤖 Generated with Claude Code