Re: [PATCH] diff: add a config option to control orderfile
On Wed, Sep 04, 2013 at 12:08:15AM +0300, Michael S. Tsirkin wrote:
Show 15 quoted lines
> On Tue, Sep 03, 2013 at 10:12:18AM -0700, Junio C Hamano wrote:
> > "Michael S. Tsirkin" <mst@redhat.com> 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 <mst@redhat.com>
> > > ---
> >
> > 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?
Ping. Any comment on this idea?
Show 28 quoted lines
> > 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);