Re: [PATCH] diff --stat: add config option to limit filename width

[Junio C Hamano <gitster@pobox.com>]

Dragan Simic <dsimic@manjaro.org> writes:

> Add new configuration option diff.statNameWidth=<width> that is equivalent
> to the command-line option --stat-name-width=<width>, but it is ignored
> by format-patch.  This follows the logic established by the already
> existing configuration option diff.statGraphWidth=<width>.
>
> Limiting the widths of names and graphs in the --stat output makes sense
> for interactive work on wide terminals with many columns, hence the support
> for these configuration options.  They don't affect format-patch because
> it already adheres to the traditional 80-column standard.
>
> Update the documentation and add more tests to cover new configuration
> option diff.statNameWidth=<width>.  While there, perform a few minor code
> and whitespace cleanups here and there, as spotted.
>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---

The stat lines have <width> (the entire display width),
<graph-width> (what appears after '|') and <name-width> (what
appears before '|'), so I would worry about letting users specify
all three to contradictory values, if there weren't an existing
command line option already.  But of course there already is a
command line option, so somebody more clever than me must have
thought about how to deal with such an impossible settings, and
adding a configuration variable to specify the same impossible
settings to the system would not make things worse.

>  Documentation/config/diff.txt  |  4 ++++
>  Documentation/diff-options.txt | 17 +++++++-------
>  builtin/diff.c                 |  1 +
>  builtin/log.c                  |  1 +
>  builtin/merge.c                |  1 +
>  builtin/rebase.c               |  1 +

Someday, as a follow-up after the dust from this topic settles, we
would probably want to look at how these rev.diffopt.* members are
initialized and refactor the common code out to a helper.  It would
allow us to instead of doing this ...

>  	/* Set up defaults that will apply to both no-index and regular diffs. */
>  	rev.diffopt.stat_width = -1;
> +	rev.diffopt.stat_name_width = -1;
>  	rev.diffopt.stat_graph_width = -1;
>  	rev.diffopt.flags.allow_external = 1;
>  	rev.diffopt.flags.allow_textconv = 1;

... in many places, do so in a single place in the helper function,
and these places will just call the helper:

	std_graph_options(&rev.diffopt);

I do not know offhand if "stat graph options related members" is a
good line to draw, or there are other things that are common outside
these .stat_foo members.  If the latter and the helper function will
initialize the members other than the stat-graph settings, its name
obviously needs a bit more thought, but you get the idae.

Thanks.