feat: add basic head command
#93
Conversation
|
|
src/shell/commands/head.rs
Outdated
| writer.write_all(b"\n")?; | ||
| lines += 1; | ||
| } else { | ||
| writer.write_all(&buf)?; |
There was a problem hiding this comment.
I think this needs to write only the size of the buffer and not the entire buffer which might contain data from a previous read at the end? We should extract out this function and add a unit test that triggers this scenario.
src/shell/commands/head.rs
Outdated
| let size = read(&mut buf)?; | ||
| if size == 0 { | ||
| break; | ||
| } else if let Some(line) = buf.split(|&b| b == b'\n').next() { |
There was a problem hiding this comment.
What happens to the rest of the lines in the buffer? I'm not sure how the tests work so I must be reading this wrong.
This patch partially addresses issue denoland#92. It implements the very basic `head` command, with the following limitations: - It can accept at most one file. - The only options it supports are `-n`/`--lines`. - The `-n`/`--lines` option cannot accept negative numbers.
|
@dsherret As you pointed out in your review, there were actually a bunch of bugs in |
|
Would there be anything left for me to fix? |
|
@dahlia no, thanks! I just need to find the time to review the actual functionality. I'll get this in for Deno 1.38 which will be released in I think a month. |
|
Thanks, I'm looking forward to it! |
|
Thank you! |
Adds a built-in `head` command to deno task: denoland/deno_task_shell#93
This patch partially addresses issue #92. It implements the very basic
headcommand, with the following limitations:-n/--lines.-n/--linesoption cannot accept negative numbers.