From: Junio C Hamano <gitster@pobox.com>
To: mmr15@case.edu
Cc: git@vger.kernel.org, Matthew Ruffalo <matthew.ruffalo@case.edu>
Subject: Re: [PATCH 2/3] diffstat: Use new diffstat config values
Date: Wed, 08 Dec 2010 21:54:07 -0800 [thread overview]
Message-ID: <7vwrnjh4ow.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1291776263-16320-2-git-send-email-matthew.ruffalo@case.edu> (mmr's message of "Tue\, 7 Dec 2010 21\:44\:22 -0500")
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().
next prev parent reply other threads:[~2010-12-09 5:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-28 23:51 [PATCH/RFC 2/3] diffstat: Use new diff.stat config values Matthew Ruffalo
2010-11-29 20:24 ` Junio C Hamano
2010-12-08 2:44 ` [PATCH 1/3] diffstat width: #define defaults in diff.h mmr15
2010-12-08 2:44 ` [PATCH 2/3] diffstat: Use new diffstat config values mmr15
2010-12-09 5:54 ` Junio C Hamano [this message]
2010-12-08 2:44 ` [PATCH 3/3] Add documentation for new diffstat config options mmr15
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vwrnjh4ow.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=matthew.ruffalo@case.edu \
--cc=mmr15@case.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).