On Sun, Sep 15, 2013 at 10:49:00AM +0300, Michael S. Tsirkin wrote:
> On Wed, Sep 04, 2013 at 12:08:15AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Sep 03, 2013 at 10:12:18AM -0700, Junio C Hamano wrote:
> > > "Michael S. Tsirkin" <m...@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 <m...@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?
Actually, after I've looked at the code,
diffcore_order is already already called for patch-id.
So patch-id shouldn't be broken by specifying the orderfile.
Why then is the feature misguided?
> > > 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);
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html