[PATCH] introduce git root

[PATCH] introduce git root

[Arjun Sreedharan]

This introduces `git root` which outputs the root directory
(the directory that contains .git).
The same can be accomplished by `git rev-parse --show-toplevel`.
`git root` is much more intuitive and easy to remember.
All it does is set the arguments for rev-parse

Signed-off-by: Arjun Sreedharan <arjun024@gmail.com>
---


Hi,
I don't know if I can just send a patch for a new "command" in git,
I probably shouldn't. Well, i thought it's anyway better explaining
this way than just asking for comments.

With the kind of projects i have been involved with in the recent past, I
have had to deal with subprojects inside projects and for many reasons had
to find ways to find the root git folder and at times to "cd" to it.

The obvious choice is to go for `git rev-parse --show-toplevel`. But, this
to me doesn't seem very _intuitive_ and `git root` does.
bzr has `bzr root`. hg has `hg root`. So, for programmers i am guessing
this pattern would also be _instinctive_, and i am thinking why not `git root`?
Arjun Sreedharan



 Makefile       |  1 +
 builtin.h      |  1 +
 builtin/root.c | 10 ++++++++++
 git.c          |  1 +
 4 files changed, 13 insertions(+)
 create mode 100644 builtin/root.c

diff --git a/Makefile b/Makefile
index 827006b..7f28d13 100644
--- a/Makefile
+++ b/Makefile
@@ -869,6 +869,7 @@ BUILTIN_OBJS += builtin/rev-list.o
 BUILTIN_OBJS += builtin/rev-parse.o
 BUILTIN_OBJS += builtin/revert.o
 BUILTIN_OBJS += builtin/rm.o
+BUILTIN_OBJS += builtin/root.o
 BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
diff --git a/builtin.h b/builtin.h
index b87df70..4672d72 100644
--- a/builtin.h
+++ b/builtin.h
@@ -112,6 +112,7 @@ extern int cmd_rev_list(int argc, const char **argv, const char *prefix);
 extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
 extern int cmd_revert(int argc, const char **argv, const char *prefix);
 extern int cmd_rm(int argc, const char **argv, const char *prefix);
+extern int cmd_root(int argc, const char **argv, const char *prefix);
 extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
diff --git a/builtin/root.c b/builtin/root.c
new file mode 100644
index 0000000..c2eeca3
--- /dev/null
+++ b/builtin/root.c
@@ -0,0 +1,10 @@
+#include "builtin.h"
+#include "argv-array.h"
+
+int cmd_root(int argc, const char **argv, const char *prefix)
+{
+	struct argv_array root_args = ARGV_ARRAY_INIT;
+
+	argv_array_pushl(&root_args, argv[0], "--show-toplevel", NULL);
+	return cmd_rev_parse(root_args.argc, root_args.argv, prefix);
+}
diff --git a/git.c b/git.c
index 18fbf79..6a0be5f 100644
--- a/git.c
+++ b/git.c
@@ -461,6 +461,7 @@ static struct cmd_struct commands[] = {
 	{ "rev-parse", cmd_rev_parse },
 	{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 	{ "rm", cmd_rm, RUN_SETUP },
+	{ "root", cmd_root, RUN_SETUP },
 	{ "send-pack", cmd_send_pack, RUN_SETUP },
 	{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
 	{ "show", cmd_show, RUN_SETUP },
-- 
1.7.11.7

Re: [PATCH] introduce git root

[Philip Oakley]

From: "Arjun Sreedharan" <arjun024@gmail.com>
> This introduces `git root` which outputs the root directory
> (the directory that contains .git).
> The same can be accomplished by `git rev-parse --show-toplevel`.
> `git root` is much more intuitive and easy to remember.
> All it does is set the arguments for rev-parse

This may be better as an alias.
I've added it to my aliases list.

>
> Signed-off-by: Arjun Sreedharan <arjun024@gmail.com>
> ---
>
>
> Hi,
> I don't know if I can just send a patch for a new "command" in git,
> I probably shouldn't. Well, i thought it's anyway better explaining
> this way than just asking for comments.
>
> With the kind of projects i have been involved with in the recent 
> past, I
> have had to deal with subprojects inside projects and for many reasons 
> had
> to find ways to find the root git folder and at times to "cd" to it.
>
> The obvious choice is to go for `git rev-parse --show-toplevel`. But, 
> this
> to me doesn't seem very _intuitive_ and `git root` does.
> bzr has `bzr root`. hg has `hg root`. So, for programmers i am 
> guessing
> this pattern would also be _instinctive_, and i am thinking why not 
> `git root`?
> Arjun Sreedharan
>
>
>
> Makefile       |  1 +
> builtin.h      |  1 +
> builtin/root.c | 10 ++++++++++
> git.c          |  1 +
> 4 files changed, 13 insertions(+)
> create mode 100644 builtin/root.c
>
> diff --git a/Makefile b/Makefile
> index 827006b..7f28d13 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -869,6 +869,7 @@ BUILTIN_OBJS += builtin/rev-list.o
> BUILTIN_OBJS += builtin/rev-parse.o
> BUILTIN_OBJS += builtin/revert.o
> BUILTIN_OBJS += builtin/rm.o
> +BUILTIN_OBJS += builtin/root.o
> BUILTIN_OBJS += builtin/send-pack.o
> BUILTIN_OBJS += builtin/shortlog.o
> BUILTIN_OBJS += builtin/show-branch.o
> diff --git a/builtin.h b/builtin.h
> index b87df70..4672d72 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -112,6 +112,7 @@ extern int cmd_rev_list(int argc, const char 
> **argv, const char *prefix);
> extern int cmd_rev_parse(int argc, const char **argv, const char 
> *prefix);
> extern int cmd_revert(int argc, const char **argv, const char 
> *prefix);
> extern int cmd_rm(int argc, const char **argv, const char *prefix);
> +extern int cmd_root(int argc, const char **argv, const char *prefix);
> extern int cmd_send_pack(int argc, const char **argv, const char 
> *prefix);
> extern int cmd_shortlog(int argc, const char **argv, const char 
> *prefix);
> extern int cmd_show(int argc, const char **argv, const char *prefix);
> diff --git a/builtin/root.c b/builtin/root.c
> new file mode 100644
> index 0000000..c2eeca3
> --- /dev/null
> +++ b/builtin/root.c
> @@ -0,0 +1,10 @@
> +#include "builtin.h"
> +#include "argv-array.h"
> +
> +int cmd_root(int argc, const char **argv, const char *prefix)
> +{
> + struct argv_array root_args = ARGV_ARRAY_INIT;
> +
> + argv_array_pushl(&root_args, argv[0], "--show-toplevel", NULL);
> + return cmd_rev_parse(root_args.argc, root_args.argv, prefix);
> +}
> diff --git a/git.c b/git.c
> index 18fbf79..6a0be5f 100644
> --- a/git.c
> +++ b/git.c
> @@ -461,6 +461,7 @@ static struct cmd_struct commands[] = {
>  { "rev-parse", cmd_rev_parse },
>  { "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
>  { "rm", cmd_rm, RUN_SETUP },
> + { "root", cmd_root, RUN_SETUP },
>  { "send-pack", cmd_send_pack, RUN_SETUP },
>  { "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
>  { "show", cmd_show, RUN_SETUP },
> -- 
> 1.7.11.7
>
> --
Philip 

Re: [PATCH] introduce git root

[Arjun Sreedharan]

On 30 November 2014 at 04:38, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Arjun Sreedharan" <arjun024@gmail.com>
>>
>> This introduces `git root` which outputs the root directory
>> (the directory that contains .git).
>> The same can be accomplished by `git rev-parse --show-toplevel`.
>> `git root` is much more intuitive and easy to remember.
>> All it does is set the arguments for rev-parse
>
>
> This may be better as an alias.
> I've added it to my aliases list.
>

I know that. I am suggesting this to be a built-in command, without having the
need to add as an alias.

>>
>> Signed-off-by: Arjun Sreedharan <arjun024@gmail.com>
>> ---
>>
>>
>> Hi,
>> I don't know if I can just send a patch for a new "command" in git,
>> I probably shouldn't. Well, i thought it's anyway better explaining
>> this way than just asking for comments.
>>
>> With the kind of projects i have been involved with in the recent past, I
>> have had to deal with subprojects inside projects and for many reasons had
>> to find ways to find the root git folder and at times to "cd" to it.
>>
>> The obvious choice is to go for `git rev-parse --show-toplevel`. But, this
>> to me doesn't seem very _intuitive_ and `git root` does.
>> bzr has `bzr root`. hg has `hg root`. So, for programmers i am guessing
>> this pattern would also be _instinctive_, and i am thinking why not `git
>> root`?
>> Arjun Sreedharan
>>
>>
>>
>> Makefile       |  1 +
>> builtin.h      |  1 +
>> builtin/root.c | 10 ++++++++++
>> git.c          |  1 +
>> 4 files changed, 13 insertions(+)
>> create mode 100644 builtin/root.c
>>
>> diff --git a/Makefile b/Makefile
>> index 827006b..7f28d13 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -869,6 +869,7 @@ BUILTIN_OBJS += builtin/rev-list.o
>> BUILTIN_OBJS += builtin/rev-parse.o
>> BUILTIN_OBJS += builtin/revert.o
>> BUILTIN_OBJS += builtin/rm.o
>> +BUILTIN_OBJS += builtin/root.o
>> BUILTIN_OBJS += builtin/send-pack.o
>> BUILTIN_OBJS += builtin/shortlog.o
>> BUILTIN_OBJS += builtin/show-branch.o
>> diff --git a/builtin.h b/builtin.h
>> index b87df70..4672d72 100644
>> --- a/builtin.h
>> +++ b/builtin.h
>> @@ -112,6 +112,7 @@ extern int cmd_rev_list(int argc, const char **argv,
>> const char *prefix);
>> extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
>> extern int cmd_revert(int argc, const char **argv, const char *prefix);
>> extern int cmd_rm(int argc, const char **argv, const char *prefix);
>> +extern int cmd_root(int argc, const char **argv, const char *prefix);
>> extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
>> extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
>> extern int cmd_show(int argc, const char **argv, const char *prefix);
>> diff --git a/builtin/root.c b/builtin/root.c
>> new file mode 100644
>> index 0000000..c2eeca3
>> --- /dev/null
>> +++ b/builtin/root.c
>> @@ -0,0 +1,10 @@
>> +#include "builtin.h"
>> +#include "argv-array.h"
>> +
>> +int cmd_root(int argc, const char **argv, const char *prefix)
>> +{
>> + struct argv_array root_args = ARGV_ARRAY_INIT;
>> +
>> + argv_array_pushl(&root_args, argv[0], "--show-toplevel", NULL);
>> + return cmd_rev_parse(root_args.argc, root_args.argv, prefix);
>> +}
>> diff --git a/git.c b/git.c
>> index 18fbf79..6a0be5f 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -461,6 +461,7 @@ static struct cmd_struct commands[] = {
>>  { "rev-parse", cmd_rev_parse },
>>  { "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
>>  { "rm", cmd_rm, RUN_SETUP },
>> + { "root", cmd_root, RUN_SETUP },
>>  { "send-pack", cmd_send_pack, RUN_SETUP },
>>  { "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
>>  { "show", cmd_show, RUN_SETUP },
>> --
>> 1.7.11.7
>>
>> --
>
> Philip

Re: [PATCH] introduce git root

[Matthieu Moy]

Arjun Sreedharan <arjun024@gmail.com> writes:

> On 30 November 2014 at 04:38, Philip Oakley <philipoakley@iee.org> wrote:
>> From: "Arjun Sreedharan" <arjun024@gmail.com>
>>>
>>> This introduces `git root` which outputs the root directory
>>> (the directory that contains .git).
>>> The same can be accomplished by `git rev-parse --show-toplevel`.
>>> `git root` is much more intuitive and easy to remember.
>>> All it does is set the arguments for rev-parse
>>
>>
>> This may be better as an alias.
>> I've added it to my aliases list.
>>
>
> I know that. I am suggesting this to be a built-in command, without having the
> need to add as an alias.

Indeed, suggesting people to add an alias does not solve the
discoverability issue. git rev-parse --show-toplevel is not just long,
it's just not the place where people would look for (it's neither about
revision nor about parsing, so clearly, "rev-parse" is not a good place
to host the feature in the UI).

If we were to rewrite Git from scratch, then I would be all for having a
"git root" command. Given that we already have rev-parse
--show-toplevel, and that we'll have to keep it anyway for backward
compatibility, I'm a bit more hesitant ("Git is hard to use because it
doesn't have enough commands" is not a complain I hear so often ;-) ),
but still mostly positive.

If we go this way, then the documentation must be updated too. I think
the doc should still recommend "git rev-parse --show-toplevel" for
scripting until Git versions implementing "git root" are widely deployed
enough.

Also, there are other options of git rev-parse which should be dealt
with: at least --show-cdup (could be eg. "git root --relative") and
--show-prefix, but probably also others from the "Options for Files" in
the man of git-rev-parse.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] introduce git root

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> ... git rev-parse --show-toplevel is not just long,
> it's just not the place where people would look for (it's neither about
> revision nor about parsing, so clearly, "rev-parse" is not a good place
> to host the feature in the UI).

For the record, "rev-parse" is not about revisions in the first
place.  It started purely as a helper to implement "git log" in
terms of "git rev-list" piped to "git diff-tree --stdin", which
requires you to sift a command line argument list given to such
a scripted "git log" into those to be passed to "rev-list" (i.e.
primarily revision ranges) and those to be passed to "diff-tree"
(i.e. primarily diff options and pathspecs).

Various subcommands "rev-parse" takes such as --show-git-dir were
added only because there wasn't any single best "kitchen sink"
command to tuck such a small feature that do not deserve a
standalone command (e.g. "git root", which makes it sound as if the
top-level of the working tree is the most important thing in the
world, and adding other useful things such as --show-prefix to it,
while it would make sense to have them there from the implementation
point of view, would be hard to justify against the connotation the
word "root" gives us), and we happened to pick "rev-parse" as a
"kitchen sink" place, which is not better or worse as anything else.

If I were redoing this today, I would probably nominate the "git"
potty as such a "kitchen synk" command.  We have "--man-path" that
shows the location of the manual pages, "--exec-path[=path]" that
either shows or allows us to override the path to the subcommands,
and "--show-prefix", "--show-toplevel", and friends may feel quite
at home there.

Re: [PATCH] introduce git root

[Christian Couder]

On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> If I were redoing this today, I would probably nominate the "git"
> potty as such a "kitchen synk" command.  We have "--man-path" that
> shows the location of the manual pages, "--exec-path[=path]" that
> either shows or allows us to override the path to the subcommands,
> and "--show-prefix", "--show-toplevel", and friends may feel quite
> at home there.

I wonder if we could reuse "git config" which is already a "kitchen
synk" command to get/set a lot of parameters.
Maybe we could dedicate a "git" or "virtual" or "proc" or "sys" (like
/proc or /sys  in Linux) namespace for these special config parameters
that would not necessarily reflect something in the config file.

"git config git.man-path" would be the same as "git --man-path".
"git config git.root" would be the same as "git rev-parse --show-toplevel".
"git config git.exec-path mypath" would allow us to override the path
to the subcommands, probably by saving something in the config file.

If we wanted for example to try just once a special exec-path we could use:

git -c git.exec-path=/path/to/git-foo foo

Best,
Christian.

Re: [PATCH] introduce git root

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> I wonder if we could reuse "git config" which is already a "kitchen
> synk" command to get/set a lot of parameters.

I doubt it makes much sense.

 * Things like toplevel and cdup are not even something you
   configure.  It is where you are, the current state of you.
   "git config" does not make any sense at all.

 * manpath and execpath and friends _might_ be something you may
   want to configure, and teach relevant codepaths to pay attention
   to the new configuration values.  But until that happens, it does
   not make sense to have that information in "git config".  "git
   config" does not show values that are not actually configured, so
   it won't be a replacement for "git --man-path" for those who do
   not have nonstandard place configured.

So...

Re: [PATCH] introduce git root

[Jeff King]

On Mon, Dec 01, 2014 at 05:17:22AM +0100, Christian Couder wrote:

> On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > If I were redoing this today, I would probably nominate the "git"
> > potty as such a "kitchen synk" command.  We have "--man-path" that
> > shows the location of the manual pages, "--exec-path[=path]" that
> > either shows or allows us to override the path to the subcommands,
> > and "--show-prefix", "--show-toplevel", and friends may feel quite
> > at home there.
> 
> I wonder if we could reuse "git config" which is already a "kitchen
> synk" command to get/set a lot of parameters.
> Maybe we could dedicate a "git" or "virtual" or "proc" or "sys" (like
> /proc or /sys  in Linux) namespace for these special config parameters
> that would not necessarily reflect something in the config file.
> 
> "git config git.man-path" would be the same as "git --man-path".
> "git config git.root" would be the same as "git rev-parse --show-toplevel".
> "git config git.exec-path mypath" would allow us to override the path
> to the subcommands, probably by saving something in the config file.

What would:

  git config git.root foo
  git config git.root

output? No matter what the answer is, I do not relish the thought of
trying to explain it in the documentation. :)

There is also "git var", which is a catch-all for printing some deduced
environmental defaults. I'd be just as happy to see it go away, though.
Having:

  git --exec-path
  git --toplevel
  git --author-ident

all work would make sense to me (I often get confused between "git
--foo" and "git rev-parse --foo" when trying to get the exec-path and
git-dir). And I don't think it's too late to move in this direction.
We'd have to keep the old interfaces around, of course, but it would
immediately improve discoverability and consistency.

-Peff

Re: [PATCH] introduce git root

[Christian Couder]

On Tue, Dec 2, 2014 at 8:04 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 01, 2014 at 05:17:22AM +0100, Christian Couder wrote:
>
>> On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> >
>> > If I were redoing this today, I would probably nominate the "git"
>> > potty as such a "kitchen synk" command.  We have "--man-path" that
>> > shows the location of the manual pages, "--exec-path[=path]" that
>> > either shows or allows us to override the path to the subcommands,
>> > and "--show-prefix", "--show-toplevel", and friends may feel quite
>> > at home there.
>>
>> I wonder if we could reuse "git config" which is already a "kitchen
>> synk" command to get/set a lot of parameters.
>> Maybe we could dedicate a "git" or "virtual" or "proc" or "sys" (like
>> /proc or /sys  in Linux) namespace for these special config parameters
>> that would not necessarily reflect something in the config file.
>>
>> "git config git.man-path" would be the same as "git --man-path".
>> "git config git.root" would be the same as "git rev-parse --show-toplevel".
>> "git config git.exec-path mypath" would allow us to override the path
>> to the subcommands, probably by saving something in the config file.
>
> What would:
>
>   git config git.root foo

That would output an error message saying that we cannot change the
git.root value.

>   git config git.root

That would output the same as "git rev-parse --show-toplevel".

> output? No matter what the answer is, I do not relish the thought of
> trying to explain it in the documentation. :)

Yeah, maybe it is better if we don't reuse "git config".

> There is also "git var", which is a catch-all for printing some deduced
> environmental defaults. I'd be just as happy to see it go away, though.

Yeah, maybe we could use "git var" for more variables.

> Having:
>
>   git --exec-path
>   git --toplevel
>   git --author-ident
>
> all work would make sense to me (I often get confused between "git
> --foo" and "git rev-parse --foo" when trying to get the exec-path and
> git-dir). And I don't think it's too late to move in this direction.
> We'd have to keep the old interfaces around, of course, but it would
> immediately improve discoverability and consistency.

I don't like reusing the git command for that puropose, because it
clutters it and makes it difficult to improve.

For example let's suppose that we decide to have a "git info" command.
It could work like this:

$ git info sequence.editor
vim

$ git info core.editor
emacs

$ git info --verbose sequence.editor
sequence.editor = vim
GIT_EDITOR = emacs
core.editor = nano

$ git info --verbose core.editor
GIT_EDITOR = emacs
core.editor = nano

So the --verbose option could explain why the sequence.editor is vim
by displaying the all the relevant options with their values from the
most important to the least important.

Best,
Christian.

Re: [PATCH] introduce git root

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> There is also "git var", which is a catch-all for printing some deduced
> environmental defaults. I'd be just as happy to see it go away, though.
> Having:
>
>   git --exec-path
>   git --toplevel
>   git --author-ident
>
> all work would make sense to me (I often get confused between "git
> --foo" and "git rev-parse --foo" when trying to get the exec-path and
> git-dir). And I don't think it's too late to move in this direction.
> We'd have to keep the old interfaces around, of course, but it would
> immediately improve discoverability and consistency.

Yeah, I too think the above makes sense.  I forgot about "var", but
it should go at the same time we move kitchen-sink options out of
"rev-parse".  One less command to worry about at the UI level means
you do not have to say "if you want to learn about X, ask 'var', if
you want to learn about Y, ask 'rev-parse', use 'git' itself for Z".

Re: [PATCH] introduce git root

[Jeff King]

On Tue, Dec 02, 2014 at 09:26:00AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > There is also "git var", which is a catch-all for printing some deduced
> > environmental defaults. I'd be just as happy to see it go away, though.
> > Having:
> >
> >   git --exec-path
> >   git --toplevel
> >   git --author-ident
> >
> > all work would make sense to me (I often get confused between "git
> > --foo" and "git rev-parse --foo" when trying to get the exec-path and
> > git-dir). And I don't think it's too late to move in this direction.
> > We'd have to keep the old interfaces around, of course, but it would
> > immediately improve discoverability and consistency.
> 
> Yeah, I too think the above makes sense.  I forgot about "var", but
> it should go at the same time we move kitchen-sink options out of
> "rev-parse".  One less command to worry about at the UI level means
> you do not have to say "if you want to learn about X, ask 'var', if
> you want to learn about Y, ask 'rev-parse', use 'git' itself for Z".

Christian raised the issue of cluttering the "git --option" namespace,
and I do agree that's a potential issue. My proposal was to drop "git
var", but I'd also be OK with making "git var" the new kitchen sink.  It
doesn't accept many options now, so it's fairly wide open for changing
without losing backwards compatibility. AFAICT, the "-l" option is
utterly useless, but other than that, it just takes a variable name. We
could introduce dashed options, or just define a sane variable namespace.

Some of the discussion has involved mixing config options into this
kitchen sink, but that does not make any sense to me (and is why I find
"git var -l" so odd). Config options are fundamentally different, in
that they are set and retrieved, not computed (from other config
variables, or from hard-coded values). And we already have a nice tool
for working with them (well...nice-ish, let's say).

-Peff

Re: [PATCH] introduce git root

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Dec 02, 2014 at 09:26:00AM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > There is also "git var", which is a catch-all for printing some deduced
>> > environmental defaults. I'd be just as happy to see it go away, though.
>> > Having:
>> >
>> >   git --exec-path
>> >   git --toplevel
>> >   git --author-ident
>> >
>> > all work would make sense to me (I often get confused between "git
>> > --foo" and "git rev-parse --foo" when trying to get the exec-path and
>> > git-dir). And I don't think it's too late to move in this direction.
>> > We'd have to keep the old interfaces around, of course, but it would
>> > immediately improve discoverability and consistency.
>> 
>> Yeah, I too think the above makes sense.  I forgot about "var", but
>> it should go at the same time we move kitchen-sink options out of
>> "rev-parse".  One less command to worry about at the UI level means
>> you do not have to say "if you want to learn about X, ask 'var', if
>> you want to learn about Y, ask 'rev-parse', use 'git' itself for Z".
>
> Christian raised the issue of cluttering the "git --option" namespace,
> and I do agree that's a potential issue. 

I am not sure if that is an issue at all.  You will need the same
number of options to cover all the necessary "computables" somewhere
anyway.

"git --show-this-or-that-computable" is not more or not less
cluttering compared to "git var --show-this-or-that-computable".

If we were to move to "git var", which takes "variables" of these
forms:

	GIT_AUTHOR_IDENT
        GIT_COMMITTER_IDENT
        GIT_EDITOR
        GIT_PAGER

then scripts that currently use "git --exec-path" need to be
encouraged to instead use "git var GIT_EXEC_PATH".  If we have so
many computables that "cluttering" may become an issue, then we
would need to come up with many new GIT_$COMPUTABLE_NAME fake
variables for consistency if we were to go with "git var", no?

I understand we are not talking about removing "git --exec-path",
but the desire is to have one single command the user can go to ask
about all the computables.  If "var" is to become that single
command, then we need to keep the interface to it uniform and
consistent, and telling the users to use "git var GIT_PAGER" and
"git var --exec-path" in the same script will not fly well.  Also
these GIT_$COMPUTABLE_NAME appear as if they can be influenced by
setting environment variables of the same name, which invites
further confusion.  This is especially bad because some of then do
get affected by environment (i.e. GIT_EDITOR="vi" has effect, but
GIT_AUTHOR_IDENT="Gitster <gitster@pobox.com>" does not).

> ... My proposal was to drop "git
> var", but I'd also be OK with making "git var" the new kitchen sink.  It
> doesn't accept many options now, so it's fairly wide open for changing
> without losing backwards compatibility. AFAICT, the "-l" option is
> utterly useless, but other than that, it just takes a variable name. We
> could introduce dashed options, or just define a sane variable namespace.

If we admit that "git var" was a failed experiment that gained only
four fake variables for the past 10 years, it will not be too much
trouble and transition pain to turn the existing ones into option
form, like --author-ident etc., like your original proposal did, I
would think.

If we are going to deprecate "git var GIT_AUTHOR_IDENT" form,
i.e. the form that uses fake variable-looking strings, and uniformly
use "git var --author-ident", "git var --exec-path", etc., then I
would think it would work, too.  I still do not know what you gain
by using "git var --exec-path" over "git --exec-path", though.

Re: [PATCH] introduce git root

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> Christian raised the issue of cluttering the "git --option"
>> namespace,
>> and I do agree that's a potential issue. 
>
> I am not sure if that is an issue at all. You will need the same
> number of options to cover all the necessary "computables" somewhere
> anyway.
>
> "git --show-this-or-that-computable" is not more or not less
> cluttering compared to "git var --show-this-or-that-computable".

I disagree.

Right now, a user reading "man git" sees --version, --help, -C,
--exec-path, --html-path, --man-path, ... at a flat list (it's actually
the first thing he can read from the man page).

The point of having commands is to make the features hierarchic. "git
rebase" do many things, but these things are all grouped under the
command "git rebase".

Indeed, I would find "git var" to be a nice place to group the "show me
such or such path". Not sure it's worth the trouble of changing the
existing "git --*-path", but I think "git var" should be the place for
new things.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] introduce git root

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> Christian raised the issue of cluttering the "git --option"
>>> namespace,
>>> and I do agree that's a potential issue. 
>>
>> I am not sure if that is an issue at all. You will need the same
>> number of options to cover all the necessary "computables" somewhere
>> anyway.
>>
>> "git --show-this-or-that-computable" is not more or not less
>> cluttering compared to "git var --show-this-or-that-computable".
>
> I disagree.
>
> Right now, a user reading "man git" sees --version, --help, -C,
> --exec-path, --html-path, --man-path, ... at a flat list (it's actually
> the first thing he can read from the man page).

Ahh, OK, you are worried about these "--give-me-computed-values"
mixed with other kinds of options.  I didn't consider that part of
the equation.

OK, "git var --exec-path" (and --friends), that is.

Re: [PATCH] introduce git root

[Jeff King]

On Thu, Dec 04, 2014 at 11:02:37AM -0800, Junio C Hamano wrote:

> > Christian raised the issue of cluttering the "git --option" namespace,
> > and I do agree that's a potential issue. 
> 
> I am not sure if that is an issue at all.  You will need the same
> number of options to cover all the necessary "computables" somewhere
> anyway.
> 
> "git --show-this-or-that-computable" is not more or not less
> cluttering compared to "git var --show-this-or-that-computable".

My issue is only that "git --foo" has other options besides computables.
So you need to name each option in a way that makes it clear it is
reporting a computable and not doing something else.

Take "git --pager" for instance. That would be a natural choice to
replace "git var GIT_PAGER". But shouldn't "--pager" be the opposite of
the existing "--no-pager"?

So instead we probably need some namespace to indicate that it is a
"showing" option. Like "--show-pager". And then for consistency, we
would probably want to move "--exec-path" to "--show-exec-path",
creating a new "--show-" namespace. Or we could call that namespace
"git var". :)

> I understand we are not talking about removing "git --exec-path",
> but the desire is to have one single command the user can go to ask
> about all the computables.  If "var" is to become that single
> command, then we need to keep the interface to it uniform and
> consistent, and telling the users to use "git var GIT_PAGER" and
> "git var --exec-path" in the same script will not fly well.  Also
> these GIT_$COMPUTABLE_NAME appear as if they can be influenced by
> setting environment variables of the same name, which invites
> further confusion.  This is especially bad because some of then do
> get affected by environment (i.e. GIT_EDITOR="vi" has effect, but
> GIT_AUTHOR_IDENT="Gitster <gitster@pobox.com>" does not).

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

and deprecating the 4 existing GIT_* variables.

> If we admit that "git var" was a failed experiment that gained only
> four fake variables for the past 10 years, it will not be too much
> trouble and transition pain to turn the existing ones into option
> form, like --author-ident etc., like your original proposal did, I
> would think.

I am also OK with that, if the details turn out to be not too ugly once
somebody starts digging in. I was just anticipating some ugliness in
advance. :) But I am not planning to work on it in the immediate future,
so whoever does can make that call.

-Peff

Re: [PATCH] introduce git root

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... I was thinking of
> just creating a new namespace, like:
>
>   git var exec-path
>   git var author-ident
>
> and deprecating the 4 existing GIT_* variables.

I'm fine with that.  As I wrote in my response to MMoy, I forgot
about other kinds of options the "git" potty takes, and "git var
name-without-needless-double-dashes" is a perfectly fine way to
consolidate the querying of computed values in a single place.

Thanks.

Re: [PATCH] introduce git root

[Christian Couder]

Jeff King <peff@peff.net> wrote:

> Some of the discussion has involved mixing config options into this
> kitchen sink, but that does not make any sense to me (and is why I
> find "git var -l" so odd). Config options are fundamentally different, in
> that they are set and retrieved, not computed (from other config
> variables, or from hard-coded values). And we already have a nice
> tool for working with them (well...nice-ish, let's say).

Yeah, but "git config" cannot say which config option applies in some
context and why.
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.

The best they can do is:

- first check if GIT_SEQUENCE_EDITOR is set, and if yes, use it
- then check if sequence.editor config variable is set, and if yes, use it
- then use "git var GIT_EDITOR" that will check the other options

I don't think it is very nice.

Jeff King <peff@peff.net> also wrote:

> My issue is only that "git --foo" has other options besides computables.
> So you need to name each option in a way that makes it clear it is
> reporting a computable and not doing something else.
>
> Take "git --pager" for instance. That would be a natural choice to
> replace "git var GIT_PAGER". But shouldn't "--pager" be the opposite of
> the existing "--no-pager"?
>
> So instead we probably need some namespace to indicate that it is a
> "showing" option. Like "--show-pager". And then for consistency, we
> would probably want to move "--exec-path" to "--show-exec-path",
> creating a new "--show-" namespace. Or we could call that namespace
> "git var". :)

I agree with that, but I think it could be better if there was also a
notion of context,

> 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

That would already be nicer than what we have now, but maybe we should
consider the following instead:

git var sequence.editor
git var core.editor

(and maybe also some aliases to core.editor, like:

git var default.editor
git var editor)

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
- they provide a notion of context.

The notion of context is interesting because suppose that we later
introduce the "commit.editor" config variable. If we decide now that
"foo.editor" means just "core.editor" if we don't know about any
"editor" variable related to the "foo" context, then the scripts that
might later be written using "git var commit.editor" will not have to
worry about the fact that previous versions of Git didn't know about
"commit.editor".

People could even start using "git var commit.editor" now, because it
would work even if "commit.editor" is unused by git commit.

Of course when the user asks for "git var foo.editor" and we don't
know about any "editor" variable related to the "foo" context, we
first should check if "foo.editor" exists in the config file and we
should use that if it exists, before we default to "git var
core.editor".

Best,
Christian.

Re: [PATCH] introduce git root

[Jeff King]

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

Re: [PATCH] introduce git root

[Christian Couder]

On Fri, Dec 5, 2014 at 10:27 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Dec 05, 2014 at 03:27:17AM +0100, Christian Couder wrote:
>
>> > 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.

Yeah but that means that we have to use something other than "-" to
separate the context from the name, because we already have names like
exec-path, html-path and man-path.

> 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.

Yeah, but I don't think it is a problem. They are different commands,
so it can be expected that they do different things.

For example, if you use "git log origin/master" you get a different
ouput than if you use "git show origin/master", though you still use
the same "origin/master" notation.

When you use "git show" you consider only the commit pointed to by
origin/master and when you use "git log" you consider the same commit
but also all its ancestors.

In the same way, when you use "git config core.editor" you consider
only the value of the core.editor logical variable in the config
files, while when you would use "git var core.editor" you would
consider the value of the core.editor logical variable in both the
config files and the environment variables.

> 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.

Using yet another namespace or syntax when we could reuse an existing
one is what would seem unnecessarily confusing to me.
The value of the "editor" logical variable in the "sequence" context
is related to the "sequence.editor" logical value in the config file,
because the later can directly influence the former. So there is a
reason to use the same notation.

Best,
Christian.

Re: [PATCH] introduce git root

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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.

I think this is a valid concern.

It is a tangent, the current definition of "git_editor" helper reads
like this:

        git_editor() {
                if test -z "${GIT_EDITOR:+set}"
                then
                        GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
                fi

                eval "$GIT_EDITOR" '"$@"'
        }

If "git var editor" were to compute a reasonable value from the
various user settings, and because GIT_EDITOR is among the sources
of user settings, I wonder if the surrounding "if test -z then...fi"
should be there.

The pager side seems to do (halfway) "the right thing":

        git_pager() {
                if test -t 1
                then
                        GIT_PAGER=$(git var GIT_PAGER)
                else
                        GIT_PAGER=cat
                fi
                : ${LESS=-FRX}
                : ${LV=-c}
                export LESS LV

                eval "$GIT_PAGER" '"$@"'
        }

The initial "test -t 1" is "we do not page to non-terminal", but we
ask "git var" to take care of PAGER/GIT_PAGER fallback/precedence.

It is tempting to argue that "we do not page to non-terminal" should
also be part of "various user settings" for "git var" to take into
account and fold this "if test -t 1...then...else...fi" also into
"git var", but because a typical command line "git var" will be used
on would look like this:

	GIT_PAGER=$(git var pager)
        eval "$GIT_PAGER" ...

with the standard output stream of "git var" not connected to
terminal, that would not fly very well, and I am not sure what
should be done about it.

This is another tangent that comes back to the original "how to name
them?" question, but I wonder if a convention like this would work.

 * When asking for a feature (e.g. "what editor to use"), if there
   is a git-specific environment variable that can be set to
   override all other settings (e.g. "$GIT_EDITOR" trumps "$EDITOR"
   environment or "core.editor" configuration), "git var" can be
   queried with the name of that "strong" environment variable.

 * All other features without such a strong environment variables
   should not be spelled as if there is such an environment variable
   the user can set in order to avoid confusion.

Here are some consequences that come to my mind, if we adopt such an
approach:

 * We would keep GIT_EDITOR and GIT_PAGER (e.g. "git var
   GIT_EDITOR");

 * GIT_SEQUENCE_EDITOR is an example of an overriding environment
   variable, and as such, "what editor to use when munging the
   rebase insn sheet?", would be "git var GIT_SEQUENCE_EDITOR".

 * We would deprecate GIT_AUTHOR_IDENT and GIT_COMMITTER_IDENT
   because setting such an environment variable would not do
   anything.  Add "git var author-ident" that is clear that it does
   not name an environment variable as its replacement.

 * It is OK to add support for new queries, e.g. GIT_AUTHOR_DATE,
   for existing environment variables that we already take into
   account but that the current "git var" does not support.

Arguably, we could go the other way around.  Instead of adding "git
var author-ident", we actually allow GIT_AUTHOR_IDENT environment
variable to be set and used instead of GIT_AUTHOR_{NAME,EMAIL,DATE}.
If that approach is practical, then we could stick to the syntax
that looks like an environment variable name.  A script could then
instead of doing:

	GIT_AUTHOR_IDENT=$(git var GIT_AUTHOR_IDENT)
        ... parse it into NAME,EMAIL,DATE and ...
        export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE

        ... complicated set of operations to prepare ...

        git commit-tree ...

do this instead:

	GIT_AUTHOR_IDENT=$(git var GIT_AUTHOR_IDENT)
        export GIT_AUTHOR_IDENT

        ... complicated set of operations to prepare ...

        git commit-tree ...

Re: [PATCH] introduce git root

[Christian Couder]

On Tue, Dec 9, 2014 at 7:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> 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.
>
> I think this is a valid concern.
>
> It is a tangent, the current definition of "git_editor" helper reads
> like this:
>
>         git_editor() {
>                 if test -z "${GIT_EDITOR:+set}"
>                 then
>                         GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
>                 fi
>
>                 eval "$GIT_EDITOR" '"$@"'
>         }
>
> If "git var editor" were to compute a reasonable value from the
> various user settings, and because GIT_EDITOR is among the sources
> of user settings, I wonder if the surrounding "if test -z then...fi"
> should be there.
>
> The pager side seems to do (halfway) "the right thing":
>
>         git_pager() {
>                 if test -t 1
>                 then
>                         GIT_PAGER=$(git var GIT_PAGER)
>                 else
>                         GIT_PAGER=cat
>                 fi
>                 : ${LESS=-FRX}
>                 : ${LV=-c}
>                 export LESS LV
>
>                 eval "$GIT_PAGER" '"$@"'
>         }
>
> The initial "test -t 1" is "we do not page to non-terminal", but we
> ask "git var" to take care of PAGER/GIT_PAGER fallback/precedence.
>
> It is tempting to argue that "we do not page to non-terminal" should
> also be part of "various user settings" for "git var" to take into
> account and fold this "if test -t 1...then...else...fi" also into
> "git var", but because a typical command line "git var" will be used
> on would look like this:
>
>         GIT_PAGER=$(git var pager)
>         eval "$GIT_PAGER" ...
>
> with the standard output stream of "git var" not connected to
> terminal, that would not fly very well, and I am not sure what
> should be done about it.
>
> This is another tangent that comes back to the original "how to name
> them?" question, but I wonder if a convention like this would work.
>
>  * When asking for a feature (e.g. "what editor to use"), if there
>    is a git-specific environment variable that can be set to
>    override all other settings (e.g. "$GIT_EDITOR" trumps "$EDITOR"
>    environment or "core.editor" configuration), "git var" can be
>    queried with the name of that "strong" environment variable.
>
>  * All other features without such a strong environment variables
>    should not be spelled as if there is such an environment variable
>    the user can set in order to avoid confusion.

Does that mean that we would also have the following:

- git var GIT_DIR
- git var GIT_EXEC_PATH
- git var GIT_HTML_PATH
- git var GIT_WORK_TREE
- git var GIT_PREFIX
...

but not:

- git var GIT_SHARED_INDEX_PATH
- git var GIT_TOP_LEVEL
- git var GIT_CDUP

?

For the above 3 variables we would have:

- git var shared-index-path
- git var top-level
- git var cdup

And then what if we add the GIT_SHARED_INDEX_PATH env variable for example?
Would we need to deprecate "git var shared-index-path"?

We also would have:

- git var GIT_EDITOR

but:

- git var web-browser (or "git var web.browser" like the config option?)

And when we add a GIT_WEB_BROWSER or GIT_BROWSER env variable, we
deprecate "git var web-browser" (or "git var web.browser"?)

I doesn't look like user and future friendly to me, but maybe I
misunderstood something.

Thanks,
Christian.

Re: [PATCH] introduce git root

[Jeff King]

On Tue, Dec 09, 2014 at 10:17:13AM -0800, Junio C Hamano wrote:

> It is a tangent, the current definition of "git_editor" helper reads
> like this:
> 
>         git_editor() {
>                 if test -z "${GIT_EDITOR:+set}"
>                 then
>                         GIT_EDITOR="$(git var GIT_EDITOR)" || return $?
>                 fi
> 
>                 eval "$GIT_EDITOR" '"$@"'
>         }
> 
> If "git var editor" were to compute a reasonable value from the
> various user settings, and because GIT_EDITOR is among the sources
> of user settings, I wonder if the surrounding "if test -z then...fi"
> should be there.

I think it should not be. The point of "git var" is to say "compute for
me what the internal C code would compute". I think it happens to be a
noop in this case, because the C code will give GIT_EDITOR preference
over anything else. But it seems like a layering violation; the shell
scripts should not have to know or care about the existence $GIT_EDITOR.

> The pager side seems to do (halfway) "the right thing":
> 
>         git_pager() {
>                 if test -t 1
>                 then
>                         GIT_PAGER=$(git var GIT_PAGER)
>                 else
>                         GIT_PAGER=cat
>                 fi
>                 : ${LESS=-FRX}
>                 : ${LV=-c}
>                 export LESS LV
> 
>                 eval "$GIT_PAGER" '"$@"'
>         }
> 
> The initial "test -t 1" is "we do not page to non-terminal", but we
> ask "git var" to take care of PAGER/GIT_PAGER fallback/precedence.
> 
> It is tempting to argue that "we do not page to non-terminal" should
> also be part of "various user settings" for "git var" to take into
> account and fold this "if test -t 1...then...else...fi" also into
> "git var", but because a typical command line "git var" will be used
> on would look like this:
> 
> 	GIT_PAGER=$(git var pager)
>         eval "$GIT_PAGER" ...
> 
> with the standard output stream of "git var" not connected to
> terminal, that would not fly very well, and I am not sure what
> should be done about it.

Right, I think in an ideal world the "is it a terminal" context is
handled by "git var", but the reality of shell scripts makes it hard. We
face the same problem with "git config --get-colorbool". There we cheat
a little and use the exit code to signal the result. That works for a
bool, but not for a string. :)

I think you'd have to do something a little gross like:

  pager=$(git var GIT_PAGER >&3) 3>&1

Except that doesn't work; the shell does not take redirections for a
straight variable assignment. Something like:

  # git var uses fd 3 by convention for stdout-tty tests
  exec 3>&1

at the top of git-sh-setup, and then:

  pager=$(git var GIT_PAGER >&3)

in the scripts themselves. That works, but is a bit ugly.

> This is another tangent that comes back to the original "how to name
> them?" question, but I wonder if a convention like this would work.
> 
>  * When asking for a feature (e.g. "what editor to use"), if there
>    is a git-specific environment variable that can be set to
>    override all other settings (e.g. "$GIT_EDITOR" trumps "$EDITOR"
>    environment or "core.editor" configuration), "git var" can be
>    queried with the name of that "strong" environment variable.

I'd prefer to create a new namespace. When you set $GIT_EDITOR as a
"trump" variable, then that is leaking information about the computation
from "git var" out to the caller. What happens if the computation
changes so that $GIT_EDITOR is no longer the last word?

>  * All other features without such a strong environment variables
>    should not be spelled as if there is such an environment variable
>    the user can set in order to avoid confusion.

So now you have two classes of variables. Some that look like
environment variables, and some that do not. What does it buy you for
the ones that do look like environment variables? I feel like either way
you will still use them as:

  editor=$(git var editor)
  eval "$editor" "$@"

Does it matter whether you call it $editor or $GIT_EDITOR?

-Peff

Re: [PATCH] introduce git root

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Dec 09, 2014 at 10:17:13AM -0800, Junio C Hamano wrote:
>
>> This is another tangent that comes back to the original "how to name
>> them?" question, but I wonder if a convention like this would work.
>> 
>>  * When asking for a feature (e.g. "what editor to use"), if there
>>    is a git-specific environment variable that can be set to
>>    override all other settings (e.g. "$GIT_EDITOR" trumps "$EDITOR"
>>    environment or "core.editor" configuration), "git var" can be
>>    queried with the name of that "strong" environment variable.
>
> I'd prefer to create a new namespace. When you set $GIT_EDITOR as a
> "trump" variable, then that is leaking information about the computation
> from "git var" out to the caller. What happens if the computation
> changes so that $GIT_EDITOR is no longer the last word?

Having two classes of variables, ones that have their corresponding
"trump" environment variables and others (and making the users aware
of the "trump" variables) is a way to teach users that they could
use them when they want to tweak the specific aspect of Git.  It of
course casts what variable trumps all others in stone, which is a
prerequisite if we want to have something stable and teachable to
the users.  Casting in stone will hinder future improvements, so the
argument cuts both ways.

Having said that, this was not even a suggestion or a proposal (in
the sense that I would be happier if people agreed on this design
rather than other design).  I'd be happy as long as people agreed on
anything sensible.

> So now you have two classes of variables. Some that look like
> environment variables, and some that do not. What does it buy you for
> the ones that do look like environment variables?

Teachability.  I personally do not care too much about it, though
;-)

I would have used a separate namespace for the ones without "trump"
variables, and would have given aliases in that namespace even for
the ones with "trump" variables, so it does not make much difference
to my mind---we will have the third namespace anyway whether we went
the "trumps can be queried via the environment variable name"
option or not.

So it seems that we agree that a separate namespace that is clearly
distinct from environment or config would be the way to go?