Denton Liu <liu.denton@xxxxxxxxx> writes: > Hi Junio, > ... >> "grep" is better only if the original were >> >> $ sed -n -e '/required match/p' >> >> but everybody would write it with grep to begin with ;-) > > This was what I was intending. It was originally written like the above > and it made sense to convert it to use grep. I guess "filter lines" in > my commit message is a little bit vague. Could we change this to "filter > matching lines" perhaps? Ah, I see. I somehow thought that some of the "sed" invocation in the original version were doing "find lines and filter its contents" (i.e. "-n -e 's/find/munge/p'"), but all three of them are just "find lines" (i.e. "-n -e '/find/p'"). So I think the change made by the patch is OK. I think I was reacting to the output of "grep" being piped to "cut". IOW, the original ... | sed -n -e '/find/p' | while read sha1 garbage do ... use sha1 ... were rewritten to ... >raw && grep -e 'find' raw | cut -d" " -f1 >orig ... use orig as a list of sha1s ... But the "grep piped to cut" can be a single process ... >raw && sed -n -e 's/\(find\).*/\1/p' raw >orig ... use orig as a list of sha1s ... So in the tiniest picture, turning "sed -n -e /find/p" into "grep" is not wrong per-se, but if you step back a bit and see a larger picture, using "sed" a bit more effectively turns out to be still a better rewrite. ... and I wrote the above before I read the remainder of your response, where you considered which one is easier to read between "grep piped to cut" and "sed" ;-) > (By the way, if I were to reroll this series, should I keep sending out > the entire patchset? It feels very noisy to send out 20-something emails > every reroll when I'm just making a small one or two line change.) Especially if it is near the end of the series, just a single step is OK. But is there anything that is glaringly wrong that needs a reroll? Or would it be "this is good enough, so let's have them cook in 'next' and graduate to 'master'---further clean-up can be done after all the dust settles"? I have an impression that we reached the latter by now. Thanks.
