Re: [PATCH 2/3] diffstat: Use new diffstat config values

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2010, 05:54 UTC
Message-ID: <7vwrnjh4ow.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1291776263-16320-2-git-send-email-matthew.ruffalo@case.edu>

mmr15@case.edu writes:

> 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.

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().