Subject: Re: [PATCH] diff: add a config option to control orderfile

From: "Michael S. Tsirkin" <[email-masked]>
Date: Wed, 4 Sep 2013 00:08:15 +0300

On Tue, Sep 03, 2013 at 10:12:18AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <[email-masked]> writes:
> 
> > I always want my diffs to show header files first,
> > then .c files, then the rest. Make it possible to
> > set orderfile though a config option to achieve this.
> >
> > Signed-off-by: Michael S. Tsirkin <[email-masked]>
> > ---
> 
> I admit that I was the guilty one who introduced the orderfile, but
> I think the feature was misguided (for one thing, it breaks the
> patch-id, I think).

This should be easy to fix - just sort before applying the ID, no?

> So I am moderately hesitant to promote its use
> with an addition like this.

The status quo just makes people insist on using a script to generate patches for
mail.  Some projects put interface documentation in the header files, in
that case it makes sense to put .h before .c in the patch as it makes
review easier.

In fact I sometimes reorder chunks in the patch manually for readability -
it's probably worth finding a way to support this.

> >  diff.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/diff.c b/diff.c
> > index 208094f..cca0767 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -264,6 +264,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
> >  		return 0;
> >  	}
> >  
> > +        if (!strcmp(var, "diff.orderfile"))
> > +                return git_config_string(&default_diff_options.orderfile, var, value);
> > +
> >  	if (!prefixcmp(var, "submodule."))
> >  		return parse_submodule_config_option(var, value);