diffstat: Use new diff.stat config values

From: Matthew Ruffalo <[email protected]>
Date: Sun, 28 Nov 2010 14:44:00 -0500
Subject: [PATCH 2/3] diffstat: Use new diff.stat config values

Previously, the diffstat width could only be specified with the command-line options '--width' and '--name-width'. This patch adds support for config file options 'diff.stat.width' and 'diff.stat.namewidth'.

The diffstat width values are obtained in this order (of increasing precedence):

1. Compile-time defaults (80 width, 50 namewidth)
2. Standard git config mechanism
3. Command-line options

This required removing the diffstat options from 'struct diff_options' and adding these values as static ints in diff.c. This preserves the style of "config options are static ints, command-line options are diff_options members". stat_opt now directly sets the global options.

---
 diff.c |   24 ++++++++++++++++++------
 diff.h |    2 --
 2 files changed, 18 insertions(+), 8 deletions(-)

index a151681..d2a3e44 100644
@@ -31,6 +31,8 @@ static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_stat_width;
+static int diff_stat_name_width;
 static struct diff_options default_diff_options;
 
 static char diff_colors[][COLOR_MAXLEN] = {

@@ -148,6 +150,16 @@ int git_diff_basic_config(const char *var, const

char *value, void *cb)

        if (!prefixcmp(var, "submodule."))
                return parse_submodule_config_option(var, value);

+ if (!strcmp(var, "diff.stat.width")) {
+ diff_stat_width = git_config_int(var, value);
+ return 0;
+ }
+
+ if (!strcmp(var, "diff.stat.namewidth")) {
+ diff_stat_name_width = git_config_int(var, value);
+ return 0;
+ }
+

        return git_color_default_config(var, value, cb);

}

@@ -1247,8 +1259,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)

                line_prefix = msg->buf;
        }

- width = options->stat_width ? options->stat_width :
DIFF_STAT_DEFAULT_WIDTH;
- name_width = options->stat_name_width ? options->stat_name_width
: DIFF_STAT_DEFAULT_NAME_WIDTH;
+ width = diff_stat_width ? diff_stat_width : DIFF_STAT_DEFAULT_WIDTH;
+ name_width = diff_stat_name_width ? diff_stat_name_width :
DIFF_STAT_DEFAULT_NAME_WIDTH;

/* Sanity: give at least 5 columns to the graph,
* but leave at least 10 columns for the name.
@@ -3053,8 +3065,8 @@ static int stat_opt(struct diff_options *options, const char **av)

 {
        const char *arg = av[0];
        char *end;
-       int width = options->stat_width;
-       int name_width = options->stat_name_width;
+       int width = diff_stat_width;
+       int name_width = diff_stat_name_width;
        int argcount = 1;
 
        arg += strlen("--stat");
@@ -3094,8 +3106,8 @@ static int stat_opt(struct diff_options *options,
const char **av)
        if (*end)
                return 0;
        options->output_format |= DIFF_FORMAT_DIFFSTAT;
-       options->stat_name_width = name_width;
-       options->stat_width = width;
+       diff_stat_name_width = name_width;
+       diff_stat_width = width;
        return argcount;

}

index 7b509c5..011f2ac 100644
@@ -122,8 +122,6 @@ struct diff_options {
        const char *stat_sep;
        long xdl_opts;
 
-       int stat_width;
-       int stat_name_width;
        const char *word_regex;
        enum diff_words_type word_diff;
 

1.7.1

Please drop these lines --- I can see them in the header of your e-mail.

In general, the second level in a three-level configuration variable name is to choose which one of unbound set of things to set the value for, and the last level of a configuration variable name is to name the specific property to affect (e.g. "difftool.<tool>.path" "remote.<name>.url").

So this sounds more like "diffstat.width" and "diffstat.namewidth". There is no set of "<something>" that share "namewidth" property to warrant the name "diff.<something>.namewidth".

The idea of the patch is good. But the message is heavily whitespace damaged and cannot be applied. Please check the settings of your MUA.

Thanks.

From: Matthew Ruffalo <[email protected]>

---
 diff.c |    4 ++--
 diff.h |    3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

index db5e844..75938e4 100644
@@ -1247,8 +1247,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		line_prefix = msg->buf;
 	}
 
-	width = options->stat_width ? options->stat_width : 80;
-	name_width = options->stat_name_width ? options->stat_name_width : 50;
+	width = options->stat_width ? options->stat_width : DIFF_STAT_DEFAULT_WIDTH;
+	name_width = options->stat_name_width ? options->stat_name_width : DIFF_STAT_DEFAULT_NAME_WIDTH;
 
 	/* Sanity: give at least 5 columns to the graph,
 	 * but leave at least 10 columns for the name.

index 0083d92..7b509c5 100644
@@ -86,6 +86,9 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
 #define DIFF_XDL_SET(opts, flag)    ((opts)->xdl_opts |= XDF_##flag)
 #define DIFF_XDL_CLR(opts, flag)    ((opts)->xdl_opts &= ~XDF_##flag)
 
+#define DIFF_STAT_DEFAULT_WIDTH       80
+#define DIFF_STAT_DEFAULT_NAME_WIDTH  50
+
 enum diff_words_type {
 	DIFF_WORDS_NONE = 0,
 	DIFF_WORDS_PORCELAIN,

1.7.1

From: Matthew Ruffalo <[email protected]>

Previously, the diffstat width could only be specified with the command-line options '--width' and '--name-width'. This patch adds support for config file options 'diffstat.width' and 'diffstat.namewidth'.

The diffstat width values are obtained in this order (of increasing precedence):

1. Compile-time defaults (80 width, 50 namewidth)
2. Standard git config mechanism
3. Command-line options

This required removing the diffstat options from 'struct diff_options' and adding these values as static ints in diff.c. This preserves the style of "config options are static ints, command-line options are diff_options members". stat_opt now directly sets the global options.

---
 diff.c |   24 ++++++++++++++++++------
 diff.h |    2 --
 2 files changed, 18 insertions(+), 8 deletions(-)

index 75938e4..d88c7f4 100644
@@ -31,6 +31,8 @@ static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_stat_width;
+static int diff_stat_name_width;
 static struct diff_options default_diff_options;
 
 static char diff_colors[][COLOR_MAXLEN] = {

@@ -148,6 +150,16 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 	if (!prefixcmp(var, "submodule."))
 		return parse_submodule_config_option(var, value);
 
+	if (!strcmp(var, "diffstat.width")) {
+		diff_stat_width = git_config_int(var, value);
+		return 0;
+	}
+
+	if (!strcmp(var, "diffstat.namewidth")) {
+		diff_stat_name_width = git_config_int(var, value);
+		return 0;
+	}
+
 	return git_color_default_config(var, value, cb);
 }
 

@@ -1247,8 +1259,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		line_prefix = msg->buf;
 	}
 
-	width = options->stat_width ? options->stat_width : DIFF_STAT_DEFAULT_WIDTH;
-	name_width = options->stat_name_width ? options->stat_name_width : DIFF_STAT_DEFAULT_NAME_WIDTH;
+	width = diff_stat_width ? diff_stat_width : DIFF_STAT_DEFAULT_WIDTH;
+	name_width = diff_stat_name_width ? diff_stat_name_width : DIFF_STAT_DEFAULT_NAME_WIDTH;
 
 	/* Sanity: give at least 5 columns to the graph,
 	 * but leave at least 10 columns for the name.

@@ -3057,8 +3069,8 @@ static int stat_opt(struct diff_options *options, const char **av)
 {
 	const char *arg = av[0];
 	char *end;
-	int width = options->stat_width;
-	int name_width = options->stat_name_width;
+	int width = diff_stat_width;
+	int name_width = diff_stat_name_width;
 	int argcount = 1;
 
 	arg += strlen("--stat");

@@ -3098,8 +3110,8 @@ static int stat_opt(struct diff_options *options, const char **av)
 	if (*end)
 		return 0;
 	options->output_format |= DIFF_FORMAT_DIFFSTAT;
-	options->stat_name_width = name_width;
-	options->stat_width = width;
+	diff_stat_name_width = name_width;
+	diff_stat_width = width;
 	return argcount;
 }
 

index 7b509c5..011f2ac 100644
@@ -122,8 +122,6 @@ struct diff_options {
 	const char *stat_sep;
 	long xdl_opts;
 
-	int stat_width;
-	int stat_name_width;
 	const char *word_regex;
 	enum diff_words_type word_diff;
 

1.7.1

It is not that I do not trust/believe it, but I am very unhappy with the above "This required".

The diff callchain was designed to be a highly reusable library and has been kept callable multiple times with different settings in a single program by passing different "struct diff_options". The above sounds like a rather huge regression.

Aren't there any way to avoid this? Why do these two options need to be any different from other variables (e.g. a_prefix, context) that can be set from the config and can be overridden by the command line options while having a built-in fallback default values?

In general, the callflow of each git subcommand looks like this:

(1) find $GIT_DIR;
(2) read $GIT_DIR/config and friends;
(3) parse command line options;
(4) decide what the user asked us to do and do it.

I would imagine that the following should do what you want:

* declare two static int variables, stat_name_width_default and stat_width_default, that are initialized to 80/50 at compile time;

* add code to git_diff_ui_config() to update these two *_default variables in step (2) above;

* add code to diff_setup() to initialize opt->stat_name_width and opt->stat_width from these two *_default variables;

* add code to diff_opt_parse() to update opt->stat_name_width and opt->stat_width from the command line parameters.

Then follow cmd_diff() in diff.c to make sure the above is sufficient. Observe that:

- The first thing cmd_diff() does is to read the config;
- then init_revisions() will call diff_setup();
- then setup_revisions() will call into diff_opt_parse().

From: Matthew Ruffalo <[email protected]>

---
 Documentation/config.txt |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

index ad5eb5f..5cae0f4 100644
@@ -862,6 +862,15 @@ diff.ignoreSubmodules::
 	commands such as 'git diff-files'. 'git checkout' also honors
 	this setting when reporting uncommitted changes.
 
+diffstat.width::
+	Controls the default width of 'git diff --stat' output. Can be
+	overridden with the command line option '--stat-width'.
+
+diffstat.namewidth::
+	Controls the default width of the filenames in 'git diff --stat'
+	output. Can be overridden with the command line option
+	'--stat-namewidth'.
+
 diff.suppressBlankEmpty::
 	A boolean to inhibit the standard behavior of printing a space
 	before each empty output line. Defaults to false.

1.7.1