Re: [PATCH] introduce git root

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Dec 9, 2014 at 7:17 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jeff King <peff@xxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]