[PATCH] Documentation: improve description of GIT_EDITOR and preference order

[PATCH] Documentation: improve description of GIT_EDITOR and preference order

[Rodrigo Silva (MestreLion)]

Previously GIT_EDITOR was not listed in git(1) "Environment Variables" section,
which could be very confusing to users. Include it in "other" subsection along
with a link to git-var(1), since that is the page that fully describes all
places where editor can be set and also their preference order.

Also, git-var(1) did not say that hardcoded fallback 'vi' may have been changed
at build time. A user could be puzzled if 'nano' pops up even when none of the
mentioned environment vars or config.editor are set. Clarify this.

Ideally, the build system should be changed to reflect the chosen fallback
editor when creating the man pages. Not sure if that is even possible though.

Signed-off-by: Rodrigo Silva (MestreLion) <linux@rodrigosilva.com>
---
 Documentation/git-var.txt |    3 ++-
 Documentation/git.txt     |    6 ++++++
 2 files changed, 8 insertions(+), 1 deletions(-)

 Granted, this is a very minor issue, but if any user stumble on this, this
 patch may reduce investigation from 50 minutes to 5

 Patch was tested in 1.7.1, but sources show this is still present in 1.7.10-rc1

diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 5317cc2..9c49163 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -43,7 +43,8 @@ GIT_EDITOR::
     `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"
     --nofork`.  The order of preference is the `$GIT_EDITOR`
     environment variable, then `core.editor` configuration, then
-    `$VISUAL`, then `$EDITOR`, and then finally 'vi'.
+    `$VISUAL`, then `$EDITOR`, and then finally a hardcoded fallback
+    editor set at build time, by default 'vi'.
 
 GIT_PAGER::
     Text viewer for use by git commands (e.g., 'less').  The value
diff --git a/Documentation/git.txt b/Documentation/git.txt
index d5b7667..fac57ba 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -711,6 +711,12 @@ other
 	a pager.  See also the `core.pager` option in
 	linkgit:git-config[1].
 
+'GIT_EDITOR'::
+    This environment variable overrides `$EDITOR` and `$VISUAL`.
+    It is used by several git comands when, on interactive mode,
+    an editor is to be launched. See also linkgit:git-var[1]
+    and the `core.editor` option in linkgit:git-config[1].
+
 'GIT_SSH'::
 	If this environment variable is set then 'git fetch'
 	and 'git push' will use this command instead
-- 
1.7.1

Re: [PATCH] Documentation: improve description of GIT_EDITOR and preference order

[Junio C Hamano]

Rodrigo Silva "(MestreLion)"  <linux@rodrigosilva.com> writes:

> Previously GIT_EDITOR was not listed in git(1) "Environment Variables" section,
> which could be very confusing to users. Include it in "other" subsection along
> with a link to git-var(1), since that is the page that fully describes all
> places where editor can be set and also their preference order.
>
> Also, git-var(1) did not say that hardcoded fallback 'vi' may have been changed
> at build time. A user could be puzzled if 'nano' pops up even when none of the
> mentioned environment vars or config.editor are set. Clarify this.
>
> Ideally, the build system should be changed to reflect the chosen fallback
> editor when creating the man pages. Not sure if that is even possible though.
>
> Signed-off-by: Rodrigo Silva (MestreLion) <linux@rodrigosilva.com>
> ---
>  Documentation/git-var.txt |    3 ++-
>  Documentation/git.txt     |    6 ++++++
>  2 files changed, 8 insertions(+), 1 deletions(-)
>
>  Granted, this is a very minor issue, but if any user stumble on this, this
>  patch may reduce investigation from 50 minutes to 5
>
>  Patch was tested in 1.7.1, but sources show this is still present in 1.7.10-rc1
>
> diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
> index 5317cc2..9c49163 100644
> --- a/Documentation/git-var.txt
> +++ b/Documentation/git-var.txt
> @@ -43,7 +43,8 @@ GIT_EDITOR::
>      `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"
>      --nofork`.  The order of preference is the `$GIT_EDITOR`
>      environment variable, then `core.editor` configuration, then
> -    `$VISUAL`, then `$EDITOR`, and then finally 'vi'.
> +    `$VISUAL`, then `$EDITOR`, and then finally a hardcoded fallback
> +    editor set at build time, by default 'vi'.

I do not think this is needed; please file a bug to whoever is replacing
'vi' with 'nono' and distributing the resulting binary, without updating
this part of the documentation.

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index d5b7667..fac57ba 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -711,6 +711,12 @@ other
>  	a pager.  See also the `core.pager` option in
>  	linkgit:git-config[1].
>  
> +'GIT_EDITOR'::
> +    This environment variable overrides `$EDITOR` and `$VISUAL`.
> +    It is used by several git comands when, on interactive mode,
> +    an editor is to be launched. See also linkgit:git-var[1]
> +    and the `core.editor` option in linkgit:git-config[1].
> +

This is a good addition.  Thanks.

>  'GIT_SSH'::
>  	If this environment variable is set then 'git fetch'
>  	and 'git push' will use this command instead

Re: [PATCH] Documentation: improve description of GIT_EDITOR and preference order

[Rodrigo Silva (MestreLion)]

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

> >
> > diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
> > index 5317cc2..9c49163 100644
> > --- a/Documentation/git-var.txt
> > +++ b/Documentation/git-var.txt
> > @@ -43,7 +43,8 @@ GIT_EDITOR::
> >      `$SOME_ENVIRONMENT_VARIABLE`, `"C:\Program Files\Vim\gvim.exe"
> >      --nofork`.  The order of preference is the `$GIT_EDITOR`
> >      environment variable, then `core.editor` configuration, then
> > -    `$VISUAL`, then `$EDITOR`, and then finally 'vi'.
> > +    `$VISUAL`, then `$EDITOR`, and then finally a hardcoded fallback
> > +    editor set at build time, by default 'vi'.
> 
> I do not think this is needed; please file a bug to whoever is replacing
> 'vi' with 'nono' and distributing the resulting binary, without updating
> this part of the documentation.

Ok, I'll do so. Just a clarification: the actual built-time replacement
value is "/usr/bin/editor", which, in my system, points to nano.

This is done in Debian and other derivatives, like Ubuntu and Linux Mint

> 
> > diff --git a/Documentation/git.txt b/Documentation/git.txt
> > index d5b7667..fac57ba 100644
> > --- a/Documentation/git.txt
> > +++ b/Documentation/git.txt
> > @@ -711,6 +711,12 @@ other
> >  	a pager.  See also the `core.pager` option in
> >  	linkgit:git-config[1].
> >  
> > +'GIT_EDITOR'::
> > +    This environment variable overrides `$EDITOR` and `$VISUAL`.
> > +    It is used by several git comands when, on interactive mode,
> > +    an editor is to be launched. See also linkgit:git-var[1]
> > +    and the `core.editor` option in linkgit:git-config[1].
> > +
> 
> This is a good addition.  Thanks.

You're welcome, always glad to help such a fabulous project.


> >  'GIT_SSH'::
> >  	If this environment variable is set then 'git fetch'
> >  	and 'git push' will use this command instead
> 


So, what should I do now? Re-send the patch with just the accepted parts
(and adjusting commit message accordingly)? Or this will be done by you?
This is my first contribution, not sure about the workflow

Thanks,
ML

Re: [PATCH] Documentation: improve description of GIT_EDITOR and preference order

[Junio C Hamano]

Rodrigo Silva "(MestreLion)"  <linux@rodrigosilva.com> writes:

> So, what should I do now? Re-send the patch with just the accepted parts
> (and adjusting commit message accordingly)? Or this will be done by you?

People either say "Yeah, I agree with the suggestions---can you apply it
without the change to git-var.txt?", or respond to the review message with
an updated patch with "I agree with the review suggestion and dropped the
change to git-var.txt" after the three-dash lines (i.e. the comment does
not become part of the commit log message).

For something small and easy like this one, it's OK to do either way.

For larger changes, people tend to do the latter, as often they do not
agree (and they do not have to) with *all* the point raised in the review,
and it would become more error prone to let me pick, choose and edit the
original submission.

I'll just apply your patch with minor tweaks.  Thanks.