Re: [PATCH 1/2] format-patch: Add a config option format.from to set the default for --from

[Josh Triplett <josh@joshtriplett.org>]

On Mon, Aug 01, 2016 at 01:32:49PM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
> 
> > +static char *from;
> >  static const char *signature = git_version_string;
> >  static const char *signature_file;
> >  static int config_cover_letter;
> > @@ -807,6 +808,17 @@ static int git_format_config(const char *var, const char *value, void *cb)
> >  		base_auto = git_config_bool(var, value);
> >  		return 0;
> >  	}
> > +	if (!strcmp(var, "format.from")) {
> > +		int b = git_config_maybe_bool(var, value);
> > +		free(from);
> > +		if (b < 0)
> > +			from = xstrdup(value);
> > +		else if (b)
> > +			from = xstrdup(git_committer_info(IDENT_NO_DATE));
> > +		else
> > +			from = NULL;
> > +		return 0;
> > +	}
> 
> One potential issue I see here is that if we ever plan to switch the
> default, we may want to issue a warning message to users who do not
> have any format.from configured when they do run the program on a
> commit that will get a different result before and after the switch
> in a release of Git before that default switch happens.  The message
> would say something like "you are formatting somebody else's commit.
> the output will change in future versions of Git and show an explicit
> in-body From: header; if you want to keep the current behaviour, set
> format.from configuration variable to false".

The previous discussion between you and Jeff King seemed to suggest that
a mention in the release notes might suffice, rather than a "noisy
deprecation" warning.

> But you cannot tell by looking at from that is NULL between two
> cases, it is NULL because we defaulted to it (in which case we do
> want to warn), or the user set it explicitly to false (we do not
> want to give the warning).

If we wanted to issue such a warning, I'd suggest a separate boolean
"from_set", set when either the configuration or command line sets
"from", and then the code that handles "from" could emit a warning to
stderr if it would produce different results and !from_set.

- Josh Triplett