Re: [PATCH] add a commit.verbose config variable
From: Matthieu Moy <hidden>
Date: 2016-06-15 23:08:41
Hi, This is v2, right? Then call it [PATCH v2] in the title, and summarize the diff wrt v1. As much as possible, Cc people who participated in the review of v1. Pranit Bauva [off-list ref] writes:
Since many people always run the command with this option, and would prefer not to use the argument again and again but instead specify it in the configuration file.
"Since ... and ..." calls for a verb to end the sentence (already noted by Eric with v1).
+commit.verbose:: + A boolean to specify whether to always include the verbose option + with `git commit`. See linkgit:git-commit[1]
Either say "See linkgit:git-commit[1]" between parenthesis, or add a period (.) at the end of the sentence.
+--override-verbose:: + Disable verbose for the commit if you have activated it + permanently in the configuration variable `commit.verbose`.
The convention is to call this --no-verbose. --override-verbose is not a good name for such option. Look how other similar cases are managed (e.g. status.short, status.branch, am.threeway)
quoted hunk
@@ -1654,6 +1656,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) argc = parse_and_validate_options(argc, argv, builtin_commit_options, builtin_commit_usage, prefix, current_head, &s); + + if( !override_verbose ) + if( !verbose )
Style: spaces between if and (, but not inside ( ).
+ git_config_get_bool("commit.verbose", &verbose);This is a strange way to write the "cli takes precedence over config". The usual way is to read from least precedence to highest precedence, i.e. git_config_get_bool before calling parse_option. You dropped the "RFC" tag from the subject line, but your patch still lacks a test. It cannot be called final without. -- Matthieu Moy http://www-verimag.imag.fr/~moy/