Re: [PATCH v2 3/4] stripspace: Implement --count-lines option

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> 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.