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