Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > Is there any application beyond git-rebase--interactive where a > --count-lines options is expected to be useful? It's not obvious from > the commit message that this change is necessarily a win for later > porting of git-rebase--interactive to C since the amount of extra code > and support material added by this patch probably outweighs the amount > of code a C version of git-rebase--interactive would need to count the > lines itself. > > Stated differently, are the two or three instances of piping through > 'wc' in git-rebase--interactive sufficient justification for > introducing extra complexity into git-stripspace and its documentation > and tests? Interesting thought. When somebody rewrites "rebase -i" in C, nobody needs to count lines in "stripspace" output. The rewritten "rebase -i" would internally run strbuf_stripspace() and the question becomes what is the best way to let that code find out how many lines the result contains. When viewed from that angle, I agree that "stripspace --count" does not add anything to further the goal of helping "rebase -i" to move to C. Adding strbuf_count_lines() that counts the number of lines in the given strbuf (if there is no such helper yet; I didn't check), though. >> +test_expect_success '--count-lines with newline only' ' >> + printf "0\n" >expect && >> + printf "\n" | git stripspace --count-lines >actual && >> + test_cmp expect actual >> +' > > What is the expected behavior when the input is an empty file, a file > with content but no newline, a file with one or more lines but lacking > a newline on the final line? Should these cases be tested, as well? Good point here, too. If we were to add strbuf_count_lines() helper, whoever adds that function needs to take a possible incomplete line at the end into account. Thanks for your comments. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html