Stefan BellerConceptually the file order as set with command line -O or via the config 'diff.orderFile' is interesting to both the author (when I run a quick git diff locally) as well as reviewer (a patch floating on the mailing list), so it is not just the author who should be responsible for getting their config in order, but a project would benefit when they could give a good default for such an order.
While the change in this RFC patch to diff.c may look uncontroversial, (Oh look! it's just another knob we can turn!), the change to the newly introduced '.gitorderfile' may be more controversial. Here is my rationale for proposing it:
I want to force myself to think about the design before pointing out
memory leaks and coding style, so the least I would wish for is:
*.h
*.c
but as we have more to look at, I would want to have the most abstract
thing to come first. And most abstract from the actual code is the
user interaction, the documentation. I heard the claim that the git
project deliberately names the directory 'Documentation/' with a capital
D such that we had this property by default already. With a patch like
this we could rename Documentation/ to docs and still enjoy reading the
docs first.
Given this alibi, I would claim that t/ is misnamed though! I personally
would prefer to review tests just after the documentation instead of
after the code as the tests are more abstract and encode promises to the
user unlike the code itself that is truth at the end of the day.Signed-off-by: Stefan Beller <sbeller@google.com> ---
I wrote:
Show 8 quoted lines
> offtopic: As a general thing for our patches, can we configure > (or even convince Git in general), that headers ought to be sent *before* > its accompanying source? I think that would help reviewers like me, who > tend to start reading linearly and then giving random thoughts, because the > header prepares the reviewer for the source code with expectations. Also > by having it the other way around, the review first focuses on design > (Is this function signature sane; the docs said it would do X while not > doing Y, is that sane?) instead of code.
and hence I came up with this patch, as I think we would want to expose such a good feature ('diff.orderFile') even for those who are not looking for it themselves.
Thanks, Stefan
.gitorderfile | 6 ++++++ diff.c | 11 +++++++++++ 2 files changed, 17 insertions(+) create mode 100644 .gitorderfile
Show changes to 2 files +17 −0
.gitorderfile, diff.c
diff --git a/.gitorderfile b/.gitorderfile new file mode 100644 index 0000000000..5131ede927 --- /dev/null +++ b/.gitorderfile @@ -0,0 +1,6 @@ +Documentation/* +t/* +*.sh +*.h +*.c +Makefile diff --git a/diff.c b/diff.c index 00b4c86698..8d537db06a 100644 --- a/diff.c +++ b/diff.c @@ -3398,6 +3398,17 @@ void diff_setup(struct diff_options *options) if (diff_indent_heuristic) DIFF_XDL_SET(options, INDENT_HEURISTIC); + if (!diff_order_file_cfg) { + struct stat st; + int c = lstat(".gitorderfile", &st); + if (c == 0 && S_ISREG(st.st_mode)) + diff_order_file_cfg = ".gitorderfile"; + else if (c < 0 && errno == ENOENT) + ; /* File does not exist. no preset. */ + else + die_errno("stat '.gitorderfile'"); + } + options->orderfile = diff_order_file_cfg; if (diff_no_prefix) {
-- 2.13.2.695.g117ddefdb4
