Apply hist --tail or --head after --grep #470

Merged
merged 1 commit into from Mar 18, 2012

Conversation

Projects
None yet
4 participants
Contributor

golive commented Feb 10, 2012

hist --grep pattern --tail N

The behaviour of --tail and --head in history command combined with --grep is not what I expected. For me, the result of combine these two options should be "the first/last N lines of history matching a pattern" but not "the lines mathing a pattern within the first/last N lines of history". I think the first use case makes more sense and is more common.

Owner

banister commented Feb 11, 2012

Thanks! will look at this soon, also @rwfitzge should have a say

Owner

rf- commented Feb 11, 2012

Looks reasonable to me. I agree that this behavior makes more sense.

Owner

rf- commented Feb 12, 2012

Actually, @banister just raised a good point, which I'd forgotten about. The reason it's like this in the first place was because having to grep through every line of history is really slow compared to taking the head/tail first. Your semantics are better, but I think it might be worth writing a fix that doesn't grep unnecessary lines.

Contributor

envygeeks commented Feb 12, 2012

I don't know if there is an easy way for Windows but for Linux you could always just pass the grep onto system grep then tail with system tail too I guess if you want (though it would probably be better to just do it as it's done now after the each_line.to_a)

Contributor

golive commented Feb 12, 2012

I didn't think about it, either, given that pry currently provides the --grep switch that searches through the entire history. My proposal was to make the same as --grep but only get the first/last N matched lines, especially the tail because it's the most recent work done.
If there is a concern for performance here, what we could do is to make stop the grep searching when N matching lines found (starting from head or reverse order with tail).
I'll make the changes if that fits you.

Owner

banister commented Feb 15, 2012

@golive yes i think that's a good idea, stop searching after matching N lines :) thanks

@rf- rf- merged commit 645821e into pry:master Mar 18, 2012

Owner

rf- commented Mar 18, 2012

I had some modifications to address the performance issue, but then I realized it was premature optimization, so I've merged your code as submitted. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment