threads / patch / 30033

patchDocumentation: improve description of GIT_EDITOR and preference order

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

## tl;dr

4 messages between Mar 23, 2012 and Mar 26, 2012. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Rodrigo Silva (MestreLion)· Mar 23, 2012, 12:38 UTC · lore

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

Show changes to 2 files +8 −1

Documentation/git-var.txt, Documentation/git.txt

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

Junio C Hamano· Mar 23, 2012, 18:18 UTC · re: Rodrigo Silva (MestreLion) · lore

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

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

Show 34 quoted lines

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

Show 14 quoted lines

> 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

Rodrigo Silva (MestreLion)· Mar 26, 2012, 08:11 UTC · re: Junio C Hamano · lore

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

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

Show 16 quoted lines

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

Show 17 quoted lines

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

Junio C Hamano· Mar 26, 2012, 18:31 UTC · re: Rodrigo Silva (MestreLion) · lore

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

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.