Adds 'head' option to allow user get the first specific number of ion values of the input file. #46
Conversation
The command should be |
Yes, this will be updated. |
src/bin/ion/commands/beta/head.rs
Outdated
| .arg( | ||
| Arg::new("number") | ||
| .long("number") | ||
| .short('n') | ||
| .allow_negative_numbers(false) | ||
| .help("Specifies the number of output top-level values.") | ||
| ) |
There was a problem hiding this comment.
This needs a default: 10. That's expected from other implementations, and simplifies error handling.
I might call this "values", not "number" but your mileage may vary. This is not user-exposed in any case. What might be more important is choosing the user-visible long-form.
See this excerpt from the man page for the macOS head implementation:
HEAD(1) General Commands Manual HEAD(1)
NAME
head – display first lines of a file
SYNOPSIS
head [-n count | -c bytes] [file ...]
DESCRIPTION
This filter displays the first count lines or bytes of each of the specified files, or of the standard input if no files
are specified. If count is omitted it defaults to 10.
The following options are available:
-c bytes, --bytes=bytes
Print bytes of each of the specified files.
-n count, --lines=count
Print count lines of each of the specified files.The important part is that the short value is -n. Given that head is normally line-oriented, having a long form of --lines makes sense. Should --number be the long form here, or would --values fit better since that's what we're counting?
There was a problem hiding this comment.
Thanks for the suggestion. I think --values will fit better in this case. This will be updated in the next commit.
src/bin/ion/commands/beta/head.rs
Outdated
| let file = File::open(input_file) | ||
| .with_context(|| format!("Could not open file '{}'", input_file))?; | ||
| if values == 0 { | ||
| return Ok(()); | ||
| } | ||
| let mut reader = ReaderBuilder::new().build(file)?; | ||
| let mut output: Box<dyn Write> = Box::new(stdout().lock()); | ||
| dump::write_all_in_format(&mut reader, &mut output, format, values).expect("Could not process the input stream."); | ||
| Ok(()) |
There was a problem hiding this comment.
Here's a crazy idea... why not just delegate to dump::run? You can even handle the values argument there, the other arguments are the same. Seems like you should be able to pass matches straight in from here to there.
Then you get all the stdin and file handling logic from dump, for free.
src/bin/ion/commands/dump.rs
Outdated
|
|
||
| // --values <n> | ||
| // this value is supplied when `dump` is invoked as `head` | ||
| let values = *matches.try_get_one::<i32>("values") |
There was a problem hiding this comment.
(Suggestion) We should probably shift head related logic to head.rs. Whatever part of dump we are reusing there we should create a helper method for that in dump.rs and then use it in head.rs.
Example:
// dump.rs
pub fn run() {
...
write_ion_values(...) // a helper method that can also be used in head.rs
...
}
// head.rs
pub fn run() {
...
// perform the values option(--values <n>) extraction here and use expect if the option is not found
write_ion_values(...)
...
}There was a problem hiding this comment.
That's a great suggestion, certainly less hacky than what we have here. I'd like to see that as a requirement for moving head out of beta, along with these two:
There was a problem hiding this comment.
… values of the input file. * Add backlog issue links corresponding to TODOs in head.rs * Add `values` parsing to dump.rs * `cargo fmt`
Issue #, if available:
N/A
Description of changes:
This PR adds an option
headtoion-clitool, which allows to get the first specific number of top-level data in the input stream.Here is the example of print out the fist 2 top-level value of testData.ion:
ion beta head --number 2 --format lines testData.ionResults:
Original data testData.ion:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.