All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Todd Zullinger <tmz@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] doc: substitute ETC_GIT(CONFIG|ATTRIBUTES) in generated docs
Date: Thu, 28 Jun 2018 14:16:29 -0400	[thread overview]
Message-ID: <20180628181629.GF31766@sigill.intra.peff.net> (raw)
In-Reply-To: <20180628163606.GP20217@zaya.teonanacatl.net>

On Thu, Jun 28, 2018 at 12:36:06PM -0400, Todd Zullinger wrote:

> >> It might be enough if the default values are relatively sane
> >> and consistent.  That would be a slight improvement over the
> >> current situation still.
> > 
> > Yeah. Taking a step back from the implementation questions, I think
> > "$(prefix)/etc/gitconfig" is not very helpful text. I'd be happy to see
> > us come up with a generic way of saying that which is more
> > comprehensible to end-users. Your patches side-step that by filling in
> > the real value, but unfortunately we can't do that everywhere. :)
> > 
> > There may not be a good "token" value, though. I.e., we may need to have
> > two sets of verbiage: the specific one, and the generic one.
> 
> Yeah.  About the best generic term I can come up with is
> using '$sysconfdir'.  But I have no idea if that's something
> most readers will recognize as a placeholder for something
> like /etc.

I don't that's much better than $(prefix). It's at least _correct_ more
often, but unless you're used to autotools conventions, it's pretty
obscure. Can we just say "the system configuration direction (e.g.,
/etc)" or something like that?

That definitely requires doing some kind of ifdef in the asciidoc
source, but I think the end result will be much more comprehensible to
end users. And IMHO the readability hit to the source is not too bad (at
least I don't find the DEFAULT_PAGER one to be).

Something like this:

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 18ddc78f42..ab20dd5235 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -113,13 +113,16 @@ For reading options: read only from global `~/.gitconfig` and from
 See also <<FILES>>.
 
 --system::
-	For writing options: write to system-wide
-	`$(prefix)/etc/gitconfig` rather than the repository
-	`.git/config`.
+	For writing options: write to the system-wide gitconfig file
+	rather then the repository config.
 +
-For reading options: read only from system-wide `$(prefix)/etc/gitconfig`
+For reading options: read only from system-wide gitconfig file
 rather than from all available files.
 +
+The exact location of the system-wide file is configured when Git is
+built.
+ifdef::git-doc-generic[In many builds, it is `/etc/gitconfig`.]
+ifndef::git-doc-generic[In your build, it is +{etc-gitconfig}+.]
 See also <<FILES>>.
 
 --local::

Though I would also be happy if we simply said "system-wide gitconfig"
here and then had the conditional part in FILES.

I'd actually argue that all of the "source" options should be grouped,
like:

  --local::
  --global::
  --system::
  --file=<file>::
	When writing, write to the repository-local, user-global, or
	system-wide configuration file (or any `<file>` you specify).
	When reading, read from a specific file rather than from all
	available files. See <<FILES> below for more details.

And then let <<FILES>> describe in prose the various files, where they
might be, etc. That also cleans up the `.git/config` thing, which is a
mild fiction (it is really `$GIT_DIR/config`).

> A number of the path references are in the FILES sections of
> the man pages.  It might not be much of an improvement if we
> try to stuff too much text in those references.  Perhaps if
> those used '$sysconfdir/gitconfig' a subsequent note could
> expand on that?  It could even be wrapped in an ifdef
> similar to that used for the default editor/pager.

So yeah, I think I am arguing along the same lines, but just saying
"system-wide gitconfig" or similar instead of $sysconfdir/gitconfig. I
think the English is a little less daunting.

> > I think it shouldn't go into config.mak.uname, since the idea there was
> > to keep the long list of platform defaults separate from other logic
> > (the Makefile is already long enough!). So I'm basically proposing the
> > same thing but in its own file. :)
> 
> Ahh, something that the top-level Makefile would create and
> then subdir Makefiles would include, perhaps similar to the
> way GIT-VERSION-FILE is updated and included?  That could
> prove useful to some of the tools in contrib which duplicate
> logic.

No, I had just envisioned it would be a static file, like
config.mak.paths or something. I'm literally just trying to break out
bits of our Makefile into bite-sized files so they're easier look at.

> Skipping that larger de-duplication cleanup, here's a stab
> at implementing the DOC_GENERIC knob (and the DOC_ overrides
> for ETC_GIT(CONFIG|ATTRIBUTES).  The defaults with
> '/GENERIC-SYSCONFDIR' in them are just placeholders waiting
> for a better name. :)

So obviously I like the direction of this patch, but if you agree with
my line of thinking above you'd need to turn DOC_GENERIC into an
attribute and use in-source conditionals. Hopefully you do agree. :)

> Thanks for thinking this through and providing some good
> directions to work on!

Thank you for working on it! I was thinking about this because of the
response I wrote in:

  https://public-inbox.org/git/20180626124316.GA15419@sigill.intra.peff.net/

If you're interested, this could all be rolled into a cleanup series.
But if not, I can wait for this to resolve and then build on top.

-Peff

  reply	other threads:[~2018-06-28 18:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-27  4:56 [PATCH] doc: substitute ETC_GIT(CONFIG|ATTRIBUTES) in generated docs Todd Zullinger
2018-06-27 14:14 ` Jeff King
2018-06-27 15:03   ` Todd Zullinger
2018-06-27 16:44     ` Todd Zullinger
2018-06-28 14:27       ` Jeff King
2018-06-28 14:15     ` Jeff King
2018-06-28 16:36       ` Todd Zullinger
2018-06-28 18:16         ` Jeff King [this message]
2018-06-27 16:45   ` Junio C Hamano
2018-06-27 20:58     ` Todd Zullinger
2018-06-28 14:28       ` Jeff King

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

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

  git send-email \
    --in-reply-to=20180628181629.GF31766@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=tmz@pobox.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.