git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: git@vger.kernel.org
Cc: jonathantanmy@google.com, Stefan Beller <sbeller@google.com>
Subject: [PATCH] RFC: Introduce '.gitorderfile'
Date: Tue, 11 Jul 2017 16:38:27 -0700	[thread overview]
Message-ID: <20170711233827.23486-1-sbeller@google.com> (raw)

Conceptually 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:
> 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

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


             reply	other threads:[~2017-07-11 23:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-11 23:38 Stefan Beller [this message]
2017-07-12 20:44 ` [PATCH] RFC: Introduce '.gitorderfile' Junio C Hamano
2017-07-12 20:57   ` Jeff King
2017-07-12 21:08     ` Stefan Beller
2017-07-13 15:59       ` Jeff King
2017-07-13 17:30         ` Stefan Beller
2017-07-13 17:32           ` Brandon Williams
2017-07-13 19:12           ` Junio C Hamano
2017-07-13 19:20             ` Stefan Beller
2017-07-13 20:47               ` Junio C Hamano
2017-07-12 23:54     ` Junio C Hamano
2017-07-13 16:00       ` Jeff King
2017-07-12 20:58   ` Stefan Beller
2017-07-12 21:37     ` Junio C Hamano
2017-07-12 21:55       ` Stefan Beller
2017-07-12 21:03   ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170711233827.23486-1-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).