git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Christian Couder <christian.couder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	Arjun Sreedharan <arjun024@gmail.com>,
	Philip Oakley <philipoakley@iee.org>, Git <git@vger.kernel.org>
Subject: Re: [PATCH] introduce git root
Date: Fri, 5 Dec 2014 04:27:52 -0500	[thread overview]
Message-ID: <20141205092752.GC32112@peff.net> (raw)
In-Reply-To: <CAP8UFD2P9P9zL=irZ-7uPD6+bEhxaiABowh0O3RT01Ov3VqT6w@mail.gmail.com>

On Fri, Dec 05, 2014 at 03:27:17AM +0100, Christian Couder wrote:

> For example, to chose the editor all the following could apply:
> 
> GIT_SEQUENCE_EDITOR env variable
> sequence.editor config variable
> GIT_EDITOR env variable
> core.editor config variable
> VISUAL env variable
> EDITOR env variable
> editor configured at compile time
> 
> and the user or our own scripts right now cannot easily know which
> editor should be used when editing the sequence list.

I think we're violently agreeing.

Taking all of those inputs and computing the final value for a
git-sequence editor is exactly what "git var" should be doing. IMHO it
is a bug that when GIT_SEQUENCE_EDITOR was introduced, there was not a
matching update to compute it from "git var" (I didn't even know
sequence.editor existed until now!).

I am not opposed to that at all; that's the point of "git var", and the
"computables" we are talking about. I am only opposed to mixing the
namespace for computables with config. I.e., there is no point in asking
"git var" for the core.editor config. It is not a computable value, and
we already have a way of accessing it (git-config, which can also
_write_ the value, something that git-var will never be able to do for
computable values).

> > I do not think "git var --exec-path" is a good idea, nor GIT_EXEC_PATH
> > for the environment-variable confusion you mentioned. I was thinking of
> > just creating a new namespace, like:
> >
> >   git var exec-path
> >   git var author-ident
> 
> I agree that this is nice, but I wonder what we would do for the
> sequence editor and the default editor.
> Maybe:
> 
> git var sequence-editor
> git var editor

Again, I think we're mostly agreeing. Context and hierarchy and falling
back are good things. Whatever we call the variables, "editor" and
"sequence-editor" and "foo-editor" should have a predictable and
consistent form. I like the idea of "foo-editor" automatically falling
back to "editor" even if we don't know what "foo" is.

But the one place I do not agree is:

> I think "sequence.editor" and "core.editor" are better because:
> 
> - they use the same syntax as the config variables, so they are easier
> to remember and to discover, and

I really don't like using "core.editor" here, because it has the same
name as a config variable, but it is _not_ the config variable. It
happens to use the config variable as one of the inputs to its
computation, but in many cases:

  git config core.editor

and

  git var core.editor

will produce different values. They are entirely different namespaces.
Using the same syntax and name seems unnecessarily confusing to me. Even
still using dotted hierarchies, but giving them different names (e.g.,
"editor", "editor.sequence", "editor.foo") would make it more obvious
that they are not the same thing.

-Peff

  reply	other threads:[~2014-12-05  9:28 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-29 20:00 [PATCH] introduce git root Arjun Sreedharan
2014-11-29 23:08 ` Philip Oakley
2014-11-30  4:35   ` Arjun Sreedharan
2014-11-30 11:58     ` Matthieu Moy
2014-12-01  3:04       ` Junio C Hamano
2014-12-01  4:17         ` Christian Couder
2014-12-01  4:53           ` Junio C Hamano
2014-12-02  7:04           ` Jeff King
2014-12-02 10:05             ` Christian Couder
2014-12-02 17:26             ` Junio C Hamano
2014-12-04  9:22               ` Jeff King
2014-12-04 19:02                 ` Junio C Hamano
2014-12-04 20:00                   ` Matthieu Moy
2014-12-04 20:19                     ` Junio C Hamano
2014-12-04 21:12                   ` Jeff King
2014-12-04 21:30                     ` Junio C Hamano
2014-12-05  2:27                     ` Christian Couder
2014-12-05  9:27                       ` Jeff King [this message]
2014-12-07  5:23                         ` Christian Couder
2014-12-09 18:17                         ` Junio C Hamano
2014-12-10  9:16                           ` Christian Couder
2014-12-10 20:10                           ` Jeff King
2014-12-10 21:08                             ` Junio C Hamano

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=20141205092752.GC32112@peff.net \
    --to=peff@peff.net \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=arjun024@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=philipoakley@iee.org \
    /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 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 NNTP newsgroup(s).