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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 17, 2015, 23:57 UTC
Message-ID: <CAPig+cQ=8FO8yFY4sHUwr0mYuyvMu4d-eizHZeadE9f0BgpXpQ@mail.gmail.com>
In-Reply-To: <1445008605-16534-4-git-send-email-tklauser@distanz.ch>

On Fri, Oct 16, 2015 at 11:16 AM, Tobias Klauser <tklauser@distanz.ch> wrote:

> Implement the --count-lines options for git stripspace [...]
>
> This will make it easier to port git-rebase--interactive.sh to C later
> on.

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?

More below.

Show 17 quoted lines

> Furthermore, add the corresponding documentation and tests.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 29e91d8..9c00cb9 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -438,4 +438,40 @@ test_expect_success 'avoid SP-HT sequence in commented line' '
>         test_cmp expect actual
>  '
>
> +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?

Show 33 quoted lines

> +test_expect_success '--count-lines with single line' '
> +       printf "1\n" >expect &&
> +       printf "foo\n" | git stripspace --count-lines >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success '--count-lines with single line preceeded by empty line' '
> +       printf "1\n" >expect &&
> +       printf "\nfoo" | git stripspace --count-lines >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success '--count-lines with single line followed by empty line' '
> +       printf "1\n" >expect &&
> +       printf "foo\n\n" | git stripspace --count-lines >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success '--count-lines with multiple lines and consecutive newlines' '
> +       printf "5\n" >expect &&
> +       printf "\none\n\n\nthree\nfour\nfive\n" | git stripspace --count-lines >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success '--count-lines combined with --strip-comments' '
> +       printf "5\n" >expect &&
> +       printf "\n# stripped\none\n#stripped\n\nthree\nfour\nfive\n" | git stripspace -s --count-lines >actual &&
> +       test_cmp expect actual
> +'
> +
>  test_done
> --
> 2.6.1.148.g7927db1