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