Skip to content

Add First filter #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 29, 2019
Merged

Add First filter #9

merged 3 commits into from
Jun 29, 2019

Conversation

bitfield
Copy link
Owner

See #7 for a discussion and use case.

@bitfield bitfield merged commit b7ef548 into master Jun 29, 2019
@bitfield bitfield deleted the first branch July 1, 2019 16:52
Copy link

@breml breml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments.

err := scanner.Err()
if err != nil {
p.SetError(err)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this implementation behave, if you pass a very big file and you use the First() filter? With the unix commands, the pipe is closed by head after 10 lines, which allows all the commands in front of head in the pipe to end as well, without the need of reading the whole file.
Would this work here as well, if we would close the pipe?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a very good point. We don't read any more of the pipe than we need to, but we also don't close it. I'd better add that...

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 8921a7d

// First reads from the pipe, and returns a new pipe containing only the first N
// lines. If there is an error reading the pipe, the pipe's error status is also
// set.
func (p *Pipe) First(lines int) *Pipe {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently negative values are treated the same as 0. head has a special interpretation of negative numbers, which could be added here as well.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GNU head allows negative arguments for -n option, meaning to print all but the last - argument value counted - lines of each input file

That sounds like effectively it would behave like Last()—well, we don't have that yet, but when we do, we won't need to worry about this :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is not like Last(), because it prints everything BUT the last n lines, where Last() prints ONLY the last n lines. So in fact it is more like an inverted Last().
A similar option is available in tail als well.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, I wasn't reading that properly. Well, it's a neat feature, but I don't have a use case for it—so I'll wait until there is one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants