On Fri, Mar 18, 2016 at 5:19 PM, Pranit Bauva <[email protected]> wrote:
> Add commit.verbose configuration variable as a convenience for those
> who always prefer --verbose.
>
> Signed-off-by: Pranit Bauva <[email protected]>
> ---
> diff --git a/builtin/commit.c b/builtin/commit.c
> @@ -1654,6 +1661,14 @@ int cmd_commit(int argc, const char **argv, const char 
> *prefix)
> +       if (verbose < 0) {
> +               if (config_verbose > -1)
> +                       verbose = config_verbose;
> +               else
> +                       verbose = 0;
> +       }

I think it's more common in this codebase to compare against -1
directly rather than <0, so:

    if (verbose == -1) {
        if (config_verbose != -1)
            verbose = config_verbose;
        else
            verbose = 0;
    }

Or, this might be easier to read:

    if (verbose == -1)
        verbose = config_verbose;

    if (verbose == -1)
        verbose = 0;

But, this likely isn't better:

    if (verbose == -1)
        verbose = config_verbose == -1 ? 0 : config_verbose;

Anyhow, probably not worth a re-roll.

> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> @@ -96,4 +96,59 @@ test_expect_success 'verbose diff is stripped out with set 
> core.commentChar' '
> +test_expect_success 'commit.verbose true and --verbose' '
> +       (
> +               GIT_EDITOR=cat &&
> +               export GIT_EDITOR &&
> +               git -c commit.verbose=true commit --amend --verbose

Easier would be to write this as:

    GIT_EDITOR=cat git -c commit.verbose=true commit --amend --verbose

and then you wouldn't need the subhsell.

However, more intuitive would probably be to create another "editor"
similar to the 'check-for-diff' editor this script already uses. (The
'check-for-diff' editor is an obvious example about how to go about
such an undertaking.) You would need to invoke 'test_set_editor' in a
subshell for this particular test in order to avoid clobbering the
global editor used by this script. Or, have a preparatory patch which
ditches the global setting of the editor and has each test invoke
'test_set_editor' as needed (and only if needed).

Same comments apply to the other new tests which use a custom "editor".

> +       ) &&
> +       grep "^diff --git" .git/COMMIT_EDITMSG >out &&
> +       wc -l out | grep "1"
> +'
> +
> +test_expect_success 'commit.verbose true and -v -v' '
> +       (
> +               GIT_EDITOR=cat &&
> +               export GIT_EDITOR &&
> +               git -c commit.verbose=true commit --amend -v -v
> +       ) &&
> +       grep "# Changes not staged for commit" .git/COMMIT_EDITMSG >out &&
> +       wc -l out | grep "2"
> +'
> +
> +test_expect_success 'commit.verbose true and --no-verbose' '
> +       test_must_fail git -c commit.verbose=true commit --amend --no-verbose
> +'
> +
> +test_expect_success 'commit.verbose false and --verbose' '
> +       git -c commit.verbose=false commit --amend --verbose
> +'
> +
> +test_expect_success 'commit.verbose false and -v -v' '
> +       (
> +               GIT_EDITOR=cat &&
> +               export GIT_EDITOR &&
> +               git -c commit.verbose=false commit --amend -v -v
> +       ) &&
> +       grep "# Changes not staged for commit" .git/COMMIT_EDITMSG >out &&
> +       wc -l out | grep "2"
> +'
> +
> +test_expect_success 'commit.verbose false and --verbose omitted' '
> +       test_must_fail git -c commit.verbose=false commit --amend
> +'
> +
> +test_expect_success 'commit.verbose false and --no-verbose' '
> +       test_must_fail git -c commit.verbose=false commit --amend --no-verbose
> +'
> +
> +test_expect_success 'status ignores commit.verbose=true' '
> +       git -c commit.verbose=true status >actual &&
> +       ! grep "^diff --git" actual
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to