feat: add file support to stacks-inspect commands #6776
Conversation
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (77.49%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6776 +/- ##
===========================================
- Coverage 78.41% 77.49% -0.92%
===========================================
Files 585 585
Lines 361849 361861 +12
===========================================
- Hits 283740 280440 -3300
- Misses 78109 81421 +3312
... and 88 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| let block_hex = &argv[2]; | ||
| let block_data = hex_bytes(block_hex).unwrap_or_else(|_| panic!("Failed to decode hex")); | ||
| let block_arg = &argv[2]; | ||
| let block_hex = if block_arg == "-" || std::path::Path::new(block_arg).exists() { |
There was a problem hiding this comment.
This approach of making the "value or filename" decision based on whether a file of said name exists feels off to me, I don't think I've ever seen that. Do you know of any precedent for this?
There was a problem hiding this comment.
Yeah that's a good point, you're right that this pattern is uncommon. My reasoning was:
- Backward compatibility: Commands like
decode-txpreviously only accepted hex strings directly. By checking file existence, existing scripts passing hex strings continue to work unchanged, while also enabling file/stdin input. - Practical safety: For hex inputs (transaction hashes, block data), it's extremely unlikely a valid hex string would collide with an existing file path. These are typically 64+ character strings like 0a1b2c3d....
- Convenience: Users don't need to remember different flags (
--filevs--hex) for a simple decode operation.
But I totally understand the concern and "extremely unlikely" is not something that I totally love either. Do you think I should add --file and --hex flags?
There was a problem hiding this comment.
You probably wouldn't need both -- if the default (backwards-compatible) behavior is a direct value, then you just need --file (or something like it) to change the semantics. So this:
stacks-inspect foo c0ff33
would be the value 0xc0ff33, and this:
stacks-inspect foo --file c0ff33
would refer to a file named c0ff33.
This does feel cleaner to me, but you have more intuition on the practical uses of this.
There was a problem hiding this comment.
Yeah that's definitely cleaner! Since I'm working on the refactoring of stacks-inspect using clap, and adding --file would require a dedicated extract_flag function or something like that, that would then be erased in favor of clap's parsing, I'd address both your comments in the clap refactoring PR
| pub fn read_optional_file_or_stdin(path: Option<&PathBuf>) -> String { | ||
| match path { | ||
| Some(p) => read_file_or_stdin(p.to_str().expect("Invalid UTF-8 in path")), | ||
| None => { |
There was a problem hiding this comment.
nit: None could defer to - handling instead of duplicating the logic
2 participants
Description
Add file and stdin input support to
stacks-inspectcommands, matching the pattern used inclarity-cli. Commands now accept:-for stdin inputApplicable issues
Additional info (benefits, drawbacks, caveats)
clarity-cliandstacks-inspect(to avoid duplication)Checklist
docs/rpc/openapi.yamlandrpc-endpoints.mdfor v2 endpoints,event-dispatcher.mdfor new events)clarity-benchmarkingrepo