Re: [PATCH v16 0/7] config commit verbose

From: Junio C Hamano <gitster@pobox.com>
Date: May 5, 2016, 19:21 UTC
Message-ID: <xmqq7ff8b99q.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1462441802-4768-1-git-send-email-pranit.bauva@gmail.com>

Pranit Bauva <pranit.bauva@gmail.com> writes:

Show 12 quoted lines

> This series of patches add a configuration variable for verbose in
> git-commit.
>
> Link to v15:
> http://thread.gmane.org/gmane.comp.version-control.git/293127
>
> Changes wrt v15:
>  * Remove the previous patch 7/7 and split the tests. Include one in
>    initial patch 6/7. The other one is introduced in a separate commit
>    after 4/7.
>  * Include tests in patch 3/6 for --no-quiet without -q, multiple verbose,
>    --no-verbose with -v as suggested by Eric Sunshine

Thanks for a pleasant read. Modulo minor readability nits I sent separately on 7/7, this looked good.

A tangent that we may want to think about after this series lands and dust settles is to make test-parse-options simpler to use. I see many instances of this repeated:

        cat >expect <<\EOF
        boolean: 0
        integer: 0
        magnitude: 0
        timestamp: 0
        string: (not set)
        abbrev: 7
        verbose: 0
        quiet: 3
        dry run: no
        file: (not set)
        EOF

        test_expect_success 'multiple quiet levels' '
                test-parse-options -q -q -q >output 2>output.err &&
                test_must_be_empty output.err &&
                test_cmp expect output
        '

But the only thing this test cares about is if "quiet: 3" is in the output. I think we should be able to write the above 18 lines with just four lines, like this:

	test_expect_success 'multiple quiet levels' '
		test-parse-options --expect="quiet: 3" -q -q -q
	'

There may be a handful of tests that care about more than one variable, and the current output format must be used when the new --expect option is not given, but I suspect that the majority of tests would want the concise form.