push.default documented in "man git-push"?

push.default documented in "man git-push"?

[David Glasser]

Is the newish push.default documented in the "git push" manpage
anywhere? I don't see it mentioned (and there are several references
to the "default" behavior), but maybe I'm missing something. Is it
left out on purpose (ie, config values aren't supposed to be mentioned
in command manpages)?

--dave

-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

David Glasser wrote:
> Is the newish push.default documented in the "git push" manpage
> anywhere? I don't see it mentioned (and there are several references
> to the "default" behavior), but maybe I'm missing something. Is it
> left out on purpose (ie, config values aren't supposed to be mentioned
> in command manpages)?

You're right.  It's documented in `man git-config`, but we should
probably mention it in the `git-push` manpage.

--8<--
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Tue, 2 Oct 2012 20:37:13 +0530
Subject: [PATCH] Documentation: mention `push.default` in git-push.txt

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-push.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..8751b38 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -37,7 +37,8 @@ OPTIONS[[OPTIONS]]
        `+`, followed by the source ref <src>, followed
        by a colon `:`, followed by the destination ref <dst>.
        It is used to specify with what <src> object the <dst> ref
-       in the remote repository is to be updated.
+       in the remote repository is to be updated.  If not specified,
+       the configuration variable `push.default` is used.
 +
 The <src> is often the name of the branch you would want to push, but
 it can be any arbitrary "SHA-1 expression", such as `master~4` or
-- 
1.7.12.1.428.g652398a.dirty

Re: push.default documented in "man git-push"?

[David Glasser]

Thanks Rankumar! There's also the reference in the "git push origin"
example and the "This is the default operation mode if no explicit
refspec is found".

(I would have sent my own patch but I can't figure out where the
syntax for the manpages is documented.)

--dave

On Tue, Oct 2, 2012 at 8:09 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> David Glasser wrote:
>> Is the newish push.default documented in the "git push" manpage
>> anywhere? I don't see it mentioned (and there are several references
>> to the "default" behavior), but maybe I'm missing something. Is it
>> left out on purpose (ie, config values aren't supposed to be mentioned
>> in command manpages)?
>
> You're right.  It's documented in `man git-config`, but we should
> probably mention it in the `git-push` manpage.
>
> --8<--
> From: Ramkumar Ramachandra <artagnon@gmail.com>
> Date: Tue, 2 Oct 2012 20:37:13 +0530
> Subject: [PATCH] Documentation: mention `push.default` in git-push.txt
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-push.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index cb97cc1..8751b38 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -37,7 +37,8 @@ OPTIONS[[OPTIONS]]
>         `+`, followed by the source ref <src>, followed
>         by a colon `:`, followed by the destination ref <dst>.
>         It is used to specify with what <src> object the <dst> ref
> -       in the remote repository is to be updated.
> +       in the remote repository is to be updated.  If not specified,
> +       the configuration variable `push.default` is used.
>  +
>  The <src> is often the name of the branch you would want to push, but
>  it can be any arbitrary "SHA-1 expression", such as `master~4` or
> --
> 1.7.12.1.428.g652398a.dirty



-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

David Glasser wrote:
> Thanks Rankumar! There's also the reference in the "git push origin"
> example and the "This is the default operation mode if no explicit
> refspec is found".

Sorry;  here's a revised patch.

--8<--
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Tue, 2 Oct 2012 21:06:05 +0530
Subject: [PATCH] Documentation: mention `push.default` in git-push.txt

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-push.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..e1e9aca 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -37,7 +37,8 @@ OPTIONS[[OPTIONS]]
        `+`, followed by the source ref <src>, followed
        by a colon `:`, followed by the destination ref <dst>.
        It is used to specify with what <src> object the <dst> ref
-       in the remote repository is to be updated.
+       in the remote repository is to be updated.  If not specified,
+       the configuration variable `push.default` is used.
 +
 The <src> is often the name of the branch you would want to push, but
 it can be any arbitrary "SHA-1 expression", such as `master~4` or
@@ -65,7 +66,8 @@ directs git to push "matching" branches: for every
branch that
 the local side, the remote side is updated if a branch of the same name
 already exists on the remote side.  This is the default operation mode
 if no explicit refspec is found (that is neither on the command line
-nor in any Push line of the corresponding remotes file---see below).
+nor in any Push line of the corresponding remotes file, or `push.default`
+---see below).

 --all::
        Instead of naming each ref to push, specifies that all
@@ -357,7 +359,7 @@ Examples
        `git push origin :`.
 +
 The default behavior of this command when no <refspec> is given can be
-configured by setting the `push` option of the remote.
+configured by setting the `push` option of the remote, or `push.default`.
 +
 For example, to default to pushing only the current branch to `origin`
 use `git config remote.origin.push HEAD`.  Any valid <refspec> (like
-- 
1.7.12.1.428.g652398a.dirty

Re: push.default documented in "man git-push"?

[Matthieu Moy]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> David Glasser wrote:
>> Thanks Rankumar! There's also the reference in the "git push origin"
>> example and the "This is the default operation mode if no explicit
>> refspec is found".
>
> Sorry;  here's a revised patch.

Sounds good, thanks (resend and Cc Junio if needed).

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

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

Matthieu Moy wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> David Glasser wrote:
>>> Thanks Rankumar! There's also the reference in the "git push origin"
>>> example and the "This is the default operation mode if no explicit
>>> refspec is found".
>>
>> Sorry;  here's a revised patch.
>
> Sounds good, thanks (resend and Cc Junio if needed).

+CC: Junio
Please add a Reported-by: David Glasser <glasser@davidglasser.net>

Ram

Re: push.default documented in "man git-push"?

[Junio C Hamano]

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

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> David Glasser wrote:
>>> Thanks Rankumar! There's also the reference in the "git push origin"
>>> example and the "This is the default operation mode if no explicit
>>> refspec is found".
>>
>> Sorry;  here's a revised patch.
>
> Sounds good, thanks (resend and Cc Junio if needed).

That's an Ack?

Re: push.default documented in "man git-push"?

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>
>>> David Glasser wrote:
>>>> Thanks Rankumar! There's also the reference in the "git push origin"
>>>> example and the "This is the default operation mode if no explicit
>>>> refspec is found".
>>>
>>> Sorry;  here's a revised patch.
>>
>> Sounds good, thanks (resend and Cc Junio if needed).
>
> That's an Ack?

Yes.

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

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> David Glasser wrote:
>> Thanks Rankumar! There's also the reference in the "git push origin"
>> example and the "This is the default operation mode if no explicit
>> refspec is found".
>
> Sorry;  here's a revised patch.
>
> --8<--

FYI: the above is not a scissors line.

> From: Ramkumar Ramachandra <artagnon@gmail.com>
> Date: Tue, 2 Oct 2012 21:06:05 +0530
> Subject: [PATCH] Documentation: mention `push.default` in git-push.txt
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-push.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index cb97cc1..e1e9aca 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -37,7 +37,8 @@ OPTIONS[[OPTIONS]]
>         `+`, followed by the source ref <src>, followed
>         by a colon `:`, followed by the destination ref <dst>.
>         It is used to specify with what <src> object the <dst> ref
> -       in the remote repository is to be updated.
> +       in the remote repository is to be updated.  If not specified,
> +       the configuration variable `push.default` is used.

I think this is way suboptimal; it is begging users to do this

	[push]
		default = frotz:xyzzy

by making it sound as if push.default gives a default for <refs>.

> @@ -65,7 +66,8 @@ directs git to push "matching" branches: for every
> branch that
>  the local side, the remote side is updated if a branch of the same name
>  already exists on the remote side.  This is the default operation mode
>  if no explicit refspec is found (that is neither on the command line
> -nor in any Push line of the corresponding remotes file---see below).
> +nor in any Push line of the corresponding remotes file, or `push.default`
> +---see below).

Likewise.  The added part should not be inside the parentheses,
which is about what the value used for refspec.  `push.default` is
about what happens when there is _no_ refspec.

I'll queue this instead.  Thanks.

-- >8 --
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Tue, 2 Oct 2012 21:08:00 +0530
Subject: [PATCH] Documentation: mention `push.default` in git-push.txt

It already is listed in the "git config" documentation, but people
interested in pushing would first look at "git push" documentation.

Noticed-by: David Glasser
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Acked-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Fixed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-push.txt | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..70b18bc 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -37,7 +37,9 @@ OPTIONS[[OPTIONS]]
 	`+`, followed by the source ref <src>, followed
 	by a colon `:`, followed by the destination ref <dst>.
 	It is used to specify with what <src> object the <dst> ref
-	in the remote repository is to be updated.
+	in the remote repository is to be updated.  If not specified,
+	the behavior of the command is controled by the `push.default`
+	configuration variable.
 +
 The <src> is often the name of the branch you would want to push, but
 it can be any arbitrary "SHA-1 expression", such as `master~4` or
@@ -65,7 +67,8 @@ directs git to push "matching" branches: for every branch that exists on
 the local side, the remote side is updated if a branch of the same name
 already exists on the remote side.  This is the default operation mode
 if no explicit refspec is found (that is neither on the command line
-nor in any Push line of the corresponding remotes file---see below).
+nor in any Push line of the corresponding remotes file---see below) and
+no `push.default` configuration variable is set.
 
 --all::
 	Instead of naming each ref to push, specifies that all
@@ -357,7 +360,8 @@ Examples
 	`git push origin :`.
 +
 The default behavior of this command when no <refspec> is given can be
-configured by setting the `push` option of the remote.
+configured by setting the `push` option of the remote, or the `push.default`
+configuration variable.
 +
 For example, to default to pushing only the current branch to `origin`
 use `git config remote.origin.push HEAD`.  Any valid <refspec> (like
-- 
1.8.0.rc0.45.g7ce8dc5

Re: push.default documented in "man git-push"?

[David Glasser]

Thanks Junio! Note that I think the word is usually spelled
"controlled" not "controled".

On Tue, Oct 2, 2012 at 11:34 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> David Glasser wrote:
>>> Thanks Rankumar! There's also the reference in the "git push origin"
>>> example and the "This is the default operation mode if no explicit
>>> refspec is found".
>>
>> Sorry;  here's a revised patch.
>>
>> --8<--
>
> FYI: the above is not a scissors line.
>
>> From: Ramkumar Ramachandra <artagnon@gmail.com>
>> Date: Tue, 2 Oct 2012 21:06:05 +0530
>> Subject: [PATCH] Documentation: mention `push.default` in git-push.txt
>>
>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>> ---
>>  Documentation/git-push.txt | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
>> index cb97cc1..e1e9aca 100644
>> --- a/Documentation/git-push.txt
>> +++ b/Documentation/git-push.txt
>> @@ -37,7 +37,8 @@ OPTIONS[[OPTIONS]]
>>         `+`, followed by the source ref <src>, followed
>>         by a colon `:`, followed by the destination ref <dst>.
>>         It is used to specify with what <src> object the <dst> ref
>> -       in the remote repository is to be updated.
>> +       in the remote repository is to be updated.  If not specified,
>> +       the configuration variable `push.default` is used.
>
> I think this is way suboptimal; it is begging users to do this
>
>         [push]
>                 default = frotz:xyzzy
>
> by making it sound as if push.default gives a default for <refs>.
>
>> @@ -65,7 +66,8 @@ directs git to push "matching" branches: for every
>> branch that
>>  the local side, the remote side is updated if a branch of the same name
>>  already exists on the remote side.  This is the default operation mode
>>  if no explicit refspec is found (that is neither on the command line
>> -nor in any Push line of the corresponding remotes file---see below).
>> +nor in any Push line of the corresponding remotes file, or `push.default`
>> +---see below).
>
> Likewise.  The added part should not be inside the parentheses,
> which is about what the value used for refspec.  `push.default` is
> about what happens when there is _no_ refspec.
>
> I'll queue this instead.  Thanks.
>
> -- >8 --
> From: Ramkumar Ramachandra <artagnon@gmail.com>
> Date: Tue, 2 Oct 2012 21:08:00 +0530
> Subject: [PATCH] Documentation: mention `push.default` in git-push.txt
>
> It already is listed in the "git config" documentation, but people
> interested in pushing would first look at "git push" documentation.
>
> Noticed-by: David Glasser
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Acked-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> Fixed-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-push.txt | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index cb97cc1..70b18bc 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -37,7 +37,9 @@ OPTIONS[[OPTIONS]]
>         `+`, followed by the source ref <src>, followed
>         by a colon `:`, followed by the destination ref <dst>.
>         It is used to specify with what <src> object the <dst> ref
> -       in the remote repository is to be updated.
> +       in the remote repository is to be updated.  If not specified,
> +       the behavior of the command is controled by the `push.default`
> +       configuration variable.
>  +
>  The <src> is often the name of the branch you would want to push, but
>  it can be any arbitrary "SHA-1 expression", such as `master~4` or
> @@ -65,7 +67,8 @@ directs git to push "matching" branches: for every branch that exists on
>  the local side, the remote side is updated if a branch of the same name
>  already exists on the remote side.  This is the default operation mode
>  if no explicit refspec is found (that is neither on the command line
> -nor in any Push line of the corresponding remotes file---see below).
> +nor in any Push line of the corresponding remotes file---see below) and
> +no `push.default` configuration variable is set.
>
>  --all::
>         Instead of naming each ref to push, specifies that all
> @@ -357,7 +360,8 @@ Examples
>         `git push origin :`.
>  +
>  The default behavior of this command when no <refspec> is given can be
> -configured by setting the `push` option of the remote.
> +configured by setting the `push` option of the remote, or the `push.default`
> +configuration variable.
>  +
>  For example, to default to pushing only the current branch to `origin`
>  use `git config remote.origin.push HEAD`.  Any valid <refspec> (like
> --
> 1.8.0.rc0.45.g7ce8dc5
>



-- 
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/

Re: push.default documented in "man git-push"?

[Junio C Hamano]

David Glasser <glasser@davidglasser.net> writes:

> Thanks Junio! Note that I think the word is usually spelled
> "controlled" not "controled".

Thanks; I cannot spelll...

Re: push.default documented in "man git-push"?

[Matthieu Moy]

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

> I'll queue this instead.  Thanks.

Even better, perfect!

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

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Tue, Oct 2, 2012 at 10:09 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> David Glasser wrote:
>> Is the newish push.default documented in the "git push" manpage
>> anywhere? I don't see it mentioned (and there are several references
>> to the "default" behavior), but maybe I'm missing something. Is it
>> left out on purpose (ie, config values aren't supposed to be mentioned
>> in command manpages)?
>
> You're right.  It's documented in `man git-config`, but we should
> probably mention it in the `git-push` manpage.

Your patch is fine. I'm just thinking whether it's a good idea to add
a section in the end of each command's man page to list all relevant
config keys to that command, somewhat similar to "see also" section.
It may help finding useful config keys that otherwise hard to find.
-- 
Duy

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

Hi Duy,

Nguyen Thai Ngoc Duy wrote:
> Your patch is fine. I'm just thinking whether it's a good idea to add
> a section in the end of each command's man page to list all relevant
> config keys to that command, somewhat similar to "see also" section.
> It may help finding useful config keys that otherwise hard to find.

That sounds like a good idea.  Would you like to write the first patch
(for git-push)?

Ram

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Wed, Oct 3, 2012 at 3:17 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> Hi Duy,
>
> Nguyen Thai Ngoc Duy wrote:
>> Your patch is fine. I'm just thinking whether it's a good idea to add
>> a section in the end of each command's man page to list all relevant
>> config keys to that command, somewhat similar to "see also" section.
>> It may help finding useful config keys that otherwise hard to find.
>
> That sounds like a good idea.  Would you like to write the first patch
> (for git-push)?

I won't be able to do it in the next 4 hours. If you want a stab at
it, go ahead.
-- 
Duy

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

[+CC: Junio, for comments]

Nguyen Thai Ngoc Duy wrote:
> On Wed, Oct 3, 2012 at 3:17 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
>> Hi Duy,
>>
>> Nguyen Thai Ngoc Duy wrote:
>>> Your patch is fine. I'm just thinking whether it's a good idea to add
>>> a section in the end of each command's man page to list all relevant
>>> config keys to that command, somewhat similar to "see also" section.
>>> It may help finding useful config keys that otherwise hard to find.
>>
>> That sounds like a good idea.  Would you like to write the first patch
>> (for git-push)?
>
> I won't be able to do it in the next 4 hours. If you want a stab at
> it, go ahead.

On second thought, it might not be such a good idea.  There are *lots*
of variables that control the operation of each command, and it's hard
to decide which ones to list and which ones to omit.  I've listed all
the relevant variables for git-push, except the advice.* variables- I
don't know how useful such a long list might be.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>

-- 8< --
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..0a1c8d2 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -426,6 +426,30 @@ Commits A and B would no longer belong to a
branch with a symbolic name,
 and so would be unreachable.  As such, these commits would be removed by
 a `git gc` command on the origin repository.

+Configuration variables
+-----------------------
+
+These configuration variables affect the operation of git-push in
+various ways.  For more information see linkgit:git-config[1].
+
+branch.<name>.remote
+core.gitProxy
+http.proxy
+push.default
+receive.autogc
+receive.denyCurrentBranch
+receive.denyDeleteCurrent
+receive.denyDeletes
+receive.denyNonFastForwards
+receive.unpackLimit
+receive.updateserverinfo
+remote.<name>.mirror
+remote.<name>.proxy
+remote.<name>.push
+remote.<name>.pushurl
+remote.<name>.receivepack
+remote.<name>.url
+
 GIT
 ---
 Part of the linkgit:git[1] suite

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> [...]
>> --8<--
>
> FYI: the above is not a scissors line.

Excerpt from git-mailinfo[1]:

--scissors::
	Remove everything in body before a scissors line.  A line that
	mainly consists of scissors (either ">8" or "8<") and perforation
	(dash "-") marks is called a scissors line, and is used to request
	the reader to cut the message at that line.  If such a line
	appears in the body of the message before the patch, everything
	before it (including the scissors line itself) is ignored when
	this option is used.

Does it need to be updated to say that a space is required between the
perforation and the scissors?

Ram

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Wed, Oct 3, 2012 at 3:46 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> On second thought, it might not be such a good idea.  There are *lots*
> of variables that control the operation of each command, and it's hard
> to decide which ones to list and which ones to omit.  I've listed all
> the relevant variables for git-push, except the advice.* variables- I
> don't know how useful such a long list might be.

I think listing receive.* and advice.* (and maybe even
remove.<name>.*) is still ok. The goal is to give users a clue.
They'll need to look up in config.txt anyway for explanation. If we
name the config keys (and groups) well then users should be able to
guess what those keys may be for before deciding whether to look into
details.
-- 
Duy

Re: push.default documented in "man git-push"?

[Peter Krefting]

Nguyen Thai Ngoc Duy:

> I'm just thinking whether it's a good idea to add a section in the 
> end of each command's man page to list all relevant config keys to 
> that command, somewhat similar to "see also" section.

Yes, please. Discoverability of configuration settings is not very 
good at the moment.

-- 
\\// Peter - http://www.softwolves.pp.se/

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Wed, Oct 3, 2012 at 3:46 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
>> On second thought, it might not be such a good idea.  There are *lots*
>> of variables that control the operation of each command, and it's hard
>> to decide which ones to list and which ones to omit.  I've listed all
>> the relevant variables for git-push, except the advice.* variables- I
>> don't know how useful such a long list might be.
>
> I think listing receive.* and advice.* (and maybe even
> remove.<name>.*) is still ok. The goal is to give users a clue.
> They'll need to look up in config.txt anyway for explanation. If we
> name the config keys (and groups) well then users should be able to
> guess what those keys may be for before deciding whether to look into
> details.

Please do not label the list as "These variables affect this
command" to give a false impression that it is the complete list if
it isn't.

Unless somebody promises to keep an up-to-date complete list there
(or even better, come up with a mechanism to help us keep that
promise automatically, perhaps by annotating pieces with structured
comments in config.txt and automatically appending such a section to
manual pages of relevant commands), that is.

With a weaker phrase, e.g. "These configuration variables may be of
interest", such a list may not hurt readers, but personally I do not
think it adds much value to have a list of variables without even a
single line description of what each is used for.

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Wed, Oct 3, 2012 at 3:46 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
>> On second thought, it might not be such a good idea.  There are *lots*
>> of variables that control the operation of each command, and it's hard
>> to decide which ones to list and which ones to omit.  I've listed all
>> the relevant variables for git-push, except the advice.* variables- I
>> don't know how useful such a long list might be.
>
> I think listing receive.* and advice.* (and maybe even
> remove.<name>.*) is still ok. The goal is to give users a clue.
> They'll need to look up in config.txt anyway for explanation. If we
> name the config keys (and groups) well then users should be able to
> guess what those keys may be for before deciding whether to look into
> details.

I would recommend against listing any advice.* in the command manual
pages.  They are meant to give an advice in cases that are often
confusing to new people and are supposed to advise how to turn it
off in the message.  We want users to see them and understand the
situation, and more importantly, we want to strongly discourage
users to decline them until seeing them and understand what they
advise for or against.

Re: push.default documented in "man git-push"?

[Jeff King]

On Wed, Oct 03, 2012 at 11:26:55AM -0700, Junio C Hamano wrote:

> Please do not label the list as "These variables affect this
> command" to give a false impression that it is the complete list if
> it isn't.
> 
> Unless somebody promises to keep an up-to-date complete list there
> (or even better, come up with a mechanism to help us keep that
> promise automatically, perhaps by annotating pieces with structured
> comments in config.txt and automatically appending such a section to
> manual pages of relevant commands), that is.
>
> With a weaker phrase, e.g. "These configuration variables may be of
> interest", such a list may not hurt readers, but personally I do not
> think it adds much value to have a list of variables without even a
> single line description of what each is used for.

We talked a while ago about actually moving the config options into the
individual manpages, and generating config.txt to simply contain an
index of keys and where their definitions may be found. That also has
the "list without description" characteristic. But presumably you would
be looking for keys in the manual of the command you want to affect, and
the master list would mostly be for redirecting you to the right
manpage.

It does break down a little when you have keys that could go in multiple
pages. In many cases, this can be solved by a canonical location that
describes the shared concepts. For example, `diff.*` should probably go
into a `gitdiff(7)` that talks about the various diff options and
formats.

Of course, that only works if you think pulling out the shared diff bits
from git-diff*, git-log, etc into a separate manpage is a good idea. I
do, because I think it makes it more clear to the reader how the
concepts connect (as opposed to simply including shared bits inline in
the manpages, as we do now, with no indication that the same content is
going to apply in many places). But it does have a downside that
individual manpages are not as easily searchable via the pager, as you
may have to follow a cross-reference to find what you want.

-Peff

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Thu, Oct 4, 2012 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I would recommend against listing any advice.* in the command manual
> pages.  They are meant to give an advice in cases that are often
> confusing to new people and are supposed to advise how to turn it
> off in the message.

OK. I think I was surprised that some messages were controlled by
advice.* but gave no hints about that and I found that out by other
means. I'll check all the advice messages.
-- 
Duy

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Thu, Oct 4, 2012 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I would recommend against listing any advice.* in the command manual
>> pages.  They are meant to give an advice in cases that are often
>> confusing to new people and are supposed to advise how to turn it
>> off in the message.
>
> OK. I think I was surprised that some messages were controlled by
> advice.* but gave no hints about that and I found that out by other
> means. I'll check all the advice messages.

As far as I can tell,

    $ git grep -e 'advice\.' Documentation

shows the list in config.txt and nothing else, and they do talk
about when they are issued, but the reasoning behind them may not be
described to a sufficient degree (that is, unless a reader carefully
thinks things through, s/he may not be able to figure out why).  But
I think what we have there is more or less OK.

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Thu, Oct 4, 2012 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Thu, Oct 4, 2012 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> I would recommend against listing any advice.* in the command manual
>>> pages.  They are meant to give an advice in cases that are often
>>> confusing to new people and are supposed to advise how to turn it
>>> off in the message.
>>
>> OK. I think I was surprised that some messages were controlled by
>> advice.* but gave no hints about that and I found that out by other
>> means. I'll check all the advice messages.
>
> As far as I can tell,
>
>     $ git grep -e 'advice\.' Documentation
>
> shows the list in config.txt and nothing else, and they do talk
> about when they are issued, but the reasoning behind them may not be
> described to a sufficient degree (that is, unless a reader carefully
> thinks things through, s/he may not be able to figure out why).  But
> I think what we have there is more or less OK.

It's about the commands, not the documents. For example I had no idea
I could set advice.statusHints to turn off some advice. "git status"
does not say that those messages can be turned off.
-- 
Duy

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Thu, Oct 4, 2012 at 12:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>>
>>> On Thu, Oct 4, 2012 at 1:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> I would recommend against listing any advice.* in the command manual
>>>> pages.  They are meant to give an advice in cases that are often
>>>> confusing to new people and are supposed to advise how to turn it
>>>> off in the message.
>>>
>>> OK. I think I was surprised that some messages were controlled by
>>> advice.* but gave no hints about that and I found that out by other
>>> means. I'll check all the advice messages.
>>
>> As far as I can tell,
>>
>>     $ git grep -e 'advice\.' Documentation
>>
>> shows the list in config.txt and nothing else, and they do talk
>> about when they are issued, but the reasoning behind them may not be
>> described to a sufficient degree (that is, unless a reader carefully
>> thinks things through, s/he may not be able to figure out why).  But
>> I think what we have there is more or less OK.
>
> It's about the commands, not the documents. For example I had no idea
> I could set advice.statusHints to turn off some advice. "git status"
> does not say that those messages can be turned off.

OK, the messages are supposed to advise how to turn them off, so we
would want some code updates in that case.

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Wed, Oct 3, 2012 at 3:46 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
>>> On second thought, it might not be such a good idea.  There are *lots*
>>> of variables that control the operation of each command, and it's hard
>>> to decide which ones to list and which ones to omit.  I've listed all
>>> the relevant variables for git-push, except the advice.* variables- I
>>> don't know how useful such a long list might be.
>>
>> I think listing receive.* and advice.* (and maybe even
>> remove.<name>.*) is still ok. The goal is to give users a clue.
>> They'll need to look up in config.txt anyway for explanation. If we
>> name the config keys (and groups) well then users should be able to
>> guess what those keys may be for before deciding whether to look into
>> details.
>
> Please do not label the list as "These variables affect this
> command" to give a false impression that it is the complete list if
> it isn't.
>
> Unless somebody promises to keep an up-to-date complete list there
> (or even better, come up with a mechanism to help us keep that
> promise automatically, perhaps by annotating pieces with structured
> comments in config.txt and automatically appending such a section to
> manual pages of relevant commands), that is.
>
> With a weaker phrase, e.g. "These configuration variables may be of
> interest", such a list may not hurt readers, but personally I do not
> think it adds much value to have a list of variables without even a
> single line description of what each is used for.

Okay.  Does this work?

-- 8< --
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Sun, 7 Oct 2012 17:24:36 +0530
Subject: [PATCH] Documentation/git-push: add a configuration variables
 section

The list of configuration variables that affect the operation of a git
command can often be hard to find.  As a first step, add a
"Configuration variables" section to git-push.  The intent is to
create similar sections in all manpages.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-push.txt | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..a125536 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -426,6 +426,30 @@ Commits A and B would no longer belong to a
branch with a sy
 and so would be unreachable.  As such, these commits would be removed by
 a `git gc` command on the origin repository.

+Configuration variables
+-----------------------
+
+These configuration variables may be of interest.  For more
+information see linkgit:git-config[1].
+
+branch.<name>.remote
+core.gitProxy
+http.proxy
+push.default
+receive.autogc
+receive.denyCurrentBranch
+receive.denyDeleteCurrent
+receive.denyDeletes
+receive.denyNonFastForwards
+receive.unpackLimit
+receive.updateserverinfo
+remote.<name>.mirror
+remote.<name>.proxy
+remote.<name>.push
+remote.<name>.pushurl
+remote.<name>.receivepack
+remote.<name>.url
+
 GIT
 ---
 Part of the linkgit:git[1] suite
-- 
1.7.12.1.428.g652398a.dirty

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>>
>> With a weaker phrase, e.g. "These configuration variables may be of
>> interest", such a list may not hurt readers, but personally I do not
>> think it adds much value to have a list of variables without even a
>> single line description of what each is used for.
>
> Okay.  Does this work?

Hrm, is it useful without even a single line description of what
each is used for?

Besides, how does it appear in the formatted documents?  A thick
paragraph that lists names of variables enumerated without comma in
between?

> -- 8< --
> From: Ramkumar Ramachandra <artagnon@gmail.com>
> Date: Sun, 7 Oct 2012 17:24:36 +0530
> Subject: [PATCH] Documentation/git-push: add a configuration variables
>  section
>
> The list of configuration variables that affect the operation of a git
> command can often be hard to find.  As a first step, add a
> "Configuration variables" section to git-push.  The intent is to
> create similar sections in all manpages.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-push.txt | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index cb97cc1..a125536 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -426,6 +426,30 @@ Commits A and B would no longer belong to a
> branch with a sy
>  and so would be unreachable.  As such, these commits would be removed by
>  a `git gc` command on the origin repository.
>
> +Configuration variables
> +-----------------------
> +
> +These configuration variables may be of interest.  For more
> +information see linkgit:git-config[1].
> +
> +branch.<name>.remote
> +core.gitProxy
> +http.proxy
> +push.default
> +receive.autogc
> +receive.denyCurrentBranch
> +receive.denyDeleteCurrent
> +receive.denyDeletes
> +receive.denyNonFastForwards
> +receive.unpackLimit
> +receive.updateserverinfo
> +remote.<name>.mirror
> +remote.<name>.proxy
> +remote.<name>.push
> +remote.<name>.pushurl
> +remote.<name>.receivepack
> +remote.<name>.url
> +
>  GIT
>  ---
>  Part of the linkgit:git[1] suite

Re: push.default documented in "man git-push"?

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>>
>>> With a weaker phrase, e.g. "These configuration variables may be of
>>> interest", such a list may not hurt readers, but personally I do not
>>> think it adds much value to have a list of variables without even a
>>> single line description of what each is used for.
>>
>> Okay.  Does this work?
>
> Hrm, is it useful without even a single line description of what
> each is used for?

Should we replicate the description from git-config[1]?  That seems
like a waste.

> Besides, how does it appear in the formatted documents?  A thick
> paragraph that lists names of variables enumerated without comma in
> between?

Oops; I didn't build it and check.

Ram

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Mon, Oct 8, 2012 at 2:27 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> Should we replicate the description from git-config[1]?  That seems
> like a waste.

I think Jeff's suggestion [1] of moving config bits from
git-config.txt to git-*.txt makes sense. But that seems a big task.
We'll need to figure out how to groups config keys in separate
config-*.txt then include them in relevant commands' man page and
config.txt (or just "see also" in config.txt). Maybe we can try it
with push config first?

[1] http://thread.gmane.org/gmane.comp.version-control.git/206780/focus=206939
-- 
Duy

Re: push.default documented in "man git-push"?

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Oct 03, 2012 at 11:26:55AM -0700, Junio C Hamano wrote:
>
>> Please do not label the list as "These variables affect this
>> command" to give a false impression that it is the complete list if
>> it isn't.
>> 
>> Unless somebody promises to keep an up-to-date complete list there
>> (or even better, come up with a mechanism to help us keep that
>> promise automatically, perhaps by annotating pieces with structured
>> comments in config.txt and automatically appending such a section to
>> manual pages of relevant commands), that is.
>>
>> With a weaker phrase, e.g. "These configuration variables may be of
>> interest", such a list may not hurt readers, but personally I do not
>> think it adds much value to have a list of variables without even a
>> single line description of what each is used for.
>
> We talked a while ago about actually moving the config options into the
> individual manpages, and generating config.txt to simply contain an
> index of keys and where their definitions may be found. That also has
> the "list without description" characteristic. But presumably you would
> be looking for keys in the manual of the command you want to affect, and
> the master list would mostly be for redirecting you to the right
> manpage.

That cuts both ways for configuration variables that affects or are
affected by multiple commands, and that would range from the trivial
example of configuration variables for fetch and merge that appear
to take effect in "git pull" from the end users' perspective, to the
more complex ones like remotes.*.merge and remotes.*.url that are
set to make the @{upstream} notation work.

> It does break down a little when you have keys that could go in multiple
> pages. In many cases, this can be solved by a canonical location that
> describes the shared concepts. For example, `diff.*` should probably go
> into a `gitdiff(7)` that talks about the various diff options and
> formats.
>
> Of course, that only works if you think pulling out the shared diff bits
> from git-diff*, git-log, etc into a separate manpage is a good idea. I
> do, because I think it makes it more clear to the reader how the
> concepts connect (as opposed to simply including shared bits inline in
> the manpages, as we do now, with no indication that the same content is
> going to apply in many places). But it does have a downside that
> individual manpages are not as easily searchable via the pager, as you
> may have to follow a cross-reference to find what you want.

I suspect that approach would make individual manual pages less
approacheable by new people, as they will have more required
reading.

git(1) currently guides people to user manual, and then points at
individual pages, assuming that these shared concepts (e.g. the one
for "diff" you discuss in the above paragraph) are fully mastered
once the user manual is read.  Perhaps we would need to replace it
with a "concepts" manual, or enhance it with more "concepts"
material?

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Fri, Oct 05, 2012 at 01:03:28PM -0700, Junio C Hamano wrote:
> >>> OK. I think I was surprised that some messages were controlled by
> >>> advice.* but gave no hints about that and I found that out by other
> >>> means. I'll check all the advice messages.
> >
> > It's about the commands, not the documents. For example I had no idea
> > I could set advice.statusHints to turn off some advice. "git status"
> > does not say that those messages can be turned off.
> 
> OK, the messages are supposed to advise how to turn them off, so we
> would want some code updates in that case.

Something like this? It turns out none of the advice messages says
anything about advice.*.

This patch makes many output more verbose/annoying, (which is good,
more motivation to turn advice off). On the other hand, if a user sees
a message telling her/him to turn advice.statusHints off, (s)he may
miss other helpful advice as it turns the whole set off. I don't know
if that is good or bad for newbies.

Tests are broken due to output changes. Will fix it if it may have a
chance of entering git.git.

There are two advise() calls in sequencer.c that we may want to
trigger with a new advice config, by the way.

-- 8< --
diff --git a/advice.c b/advice.c
index edfbd4a..5b78c01 100644
--- a/advice.c
+++ b/advice.c
@@ -70,7 +70,8 @@ int error_resolve_conflict(const char *me)
 		advise(_("Fix them up in the work tree,\n"
 			 "and then use 'git add/rm <file>' as\n"
 			 "appropriate to mark resolution and make a commit,\n"
-			 "or use 'git commit -a'."));
+			 "or use 'git commit -a'.\n"
+			 "Set advice.resolveConflict to false to turn off this message."));
 	return -1;
 }
 
@@ -89,7 +90,9 @@ void detach_advice(const char *new_name)
 	"state without impacting any branches by performing another checkout.\n\n"
 	"If you want to create a new branch to retain commits you create, you may\n"
 	"do so (now or later) by using -b with the checkout command again. Example:\n\n"
-	"  git checkout -b new_branch_name\n\n";
+	"  git checkout -b new_branch_name\n"
+	"Set either advice.detachedHead to false to turn off this message\n\n"
+;
 
 	fprintf(stderr, fmt, new_name);
 }
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..a805961 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -699,7 +699,9 @@ static void suggest_reattach(struct commit *commit, struct rev_info *revs)
 			_(
 			"If you want to keep them by creating a new branch, "
 			"this may be a good time\nto do so with:\n\n"
-			" git branch new_branch_name %s\n\n"),
+			" git branch new_branch_name %s\n"
+			"Set either advice.detachedHead to false"
+			" to turn off this message\n\n"),
 			sha1_to_hex(commit->object.sha1));
 }
 
diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..5a4d85f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -50,7 +50,8 @@ N_("Your name and email address were configured automatically based\n"
 "\n"
 "After doing this, you may fix the identity used for this commit with:\n"
 "\n"
-"    git commit --amend --reset-author\n");
+"    git commit --amend --reset-author\n"
+"Set either advice.implicitIdentity to false to turn off this message\n");
 
 static const char empty_amend_advice[] =
 N_("You asked to amend the most recent commit, but doing so would make\n"
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ec8f0d..754f51e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1227,14 +1227,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 */
 		if (advice_resolve_conflict)
 			die(_("You have not concluded your merge (MERGE_HEAD exists).\n"
-				  "Please, commit your changes before you can merge."));
+			      "Please, commit your changes before you can merge.\n"
+			      "Set advice.resolveConflict to false"
+			      " to turn off this message\n"));
 		else
 			die(_("You have not concluded your merge (MERGE_HEAD exists)."));
 	}
 	if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
 		if (advice_resolve_conflict)
 			die(_("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n"
-			    "Please, commit your changes before you can merge."));
+			    "Please, commit your changes before you can merge.\n"
+			      "Set advice.resolveConflict to false"
+			      " to turn off this message\n"));
 		else
 			die(_("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists)."));
 	}
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..420a17a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -206,19 +206,25 @@ static const char message_advice_pull_before_push[] =
 	N_("Updates were rejected because the tip of your current branch is behind\n"
 	   "its remote counterpart. Merge the remote changes (e.g. 'git pull')\n"
 	   "before pushing again.\n"
-	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
+	   "See the 'Note about fast-forwards' in 'git push --help' for details.\n"
+	   "Set either advice.pushNonFFCurrent or advice.pushNonFostForward to false\n"
+	   " to turn off this message");
 
 static const char message_advice_use_upstream[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. If you did not intend to push that branch, you may want to\n"
 	   "specify branches to push or set the 'push.default' configuration variable\n"
-	   "to 'simple', 'current' or 'upstream' to push only the current branch.");
+	   "to 'simple', 'current' or 'upstream' to push only the current branch.\n"
+	   "Set either advice.pushNonFFDefault or advice.pushNonFostForward to false\n"
+	   " to turn off this message");
 
 static const char message_advice_checkout_pull_push[] =
 	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
 	   "counterpart. Check out this branch and merge the remote changes\n"
 	   "(e.g. 'git pull') before pushing again.\n"
-	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
+	   "See the 'Note about fast-forwards' in 'git push --help' for details.\n"
+	   "Set either advice.pushNonFFMatching or advice.pushNonFostForward to false\n"
+	   " to turn off this message");
 
 static void advise_pull_before_push(void)
 {
diff --git a/notes-merge.c b/notes-merge.c
index 0f67bd3..601bf26 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -273,7 +273,10 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
 				    "'git notes merge --commit' or 'git notes "
 				    "merge --abort' to commit/abort the "
 				    "previous merge before you start a new "
-				    "notes merge.", git_path("NOTES_MERGE_*"));
+				    "notes merge.\n"
+				    "Set advice.resolveConflict to false"
+				    " to turn off this message",
+				    git_path("NOTES_MERGE_*"));
 			else
 				die("You have not concluded your notes merge "
 				    "(%s exists).", git_path("NOTES_MERGE_*"));
diff --git a/sequencer.c b/sequencer.c
index e3723d2..489c97c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -182,7 +182,8 @@ static int error_dirty_index(struct replay_opts *opts)
 		error(_("Your local changes would be overwritten by revert."));
 
 	if (advice_commit_before_merge)
-		advise(_("Commit your changes or stash them to proceed."));
+		advise(_("Commit your changes or stash them to proceed.\n"
+			 "Set advice.commitBeforeMerge to false to turn off this message."));
 	return -1;
 }
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 6d96366..6c274bc 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -60,7 +60,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 	const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
 	if (advice_commit_before_merge)
 		msg = "Your local changes to the following files would be overwritten by %s:\n%%s"
-			"Please, commit your changes or stash them before you can %s.";
+			"Please, commit your changes or stash them before you can %s.\n"
+			"Set advice.commitBeforeMerge to false to turn off this message.";
 	else
 		msg = "Your local changes to the following files would be overwritten by %s:\n%%s";
 	tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2);
@@ -73,7 +74,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 
 	if (advice_commit_before_merge)
 		msg = "The following untracked working tree files would be %s by %s:\n%%s"
-			"Please move or remove them before you can %s.";
+			"Please move or remove them before you can %s.\n"
+			"Set advice.commitBeforeMerge to false to turn off this message.";
 	else
 		msg = "The following untracked working tree files would be %s by %s:\n%%s";
 	tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("removed") + strlen(cmd2) - 4);
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..344f5f6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -178,6 +178,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 	} else {
 		status_printf_ln(s, c, _("  (use \"git add/rm <file>...\" as appropriate to mark resolution)"));
 	}
+	status_printf_ln(s, c, _("  Set advice.statusHints to false to turn off this message"));
 	status_printf_ln(s, c, "");
 }
 
@@ -194,6 +195,7 @@ static void wt_status_print_cached_header(struct wt_status *s)
 		status_printf_ln(s, c, _("  (use \"git reset %s <file>...\" to unstage)"), s->reference);
 	else
 		status_printf_ln(s, c, _("  (use \"git rm --cached <file>...\" to unstage)"));
+	status_printf_ln(s, c, _("  Set advice.statusHints to false to turn off this message"));
 	status_printf_ln(s, c, "");
 }
 
@@ -213,6 +215,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 	status_printf_ln(s, c, _("  (use \"git checkout -- <file>...\" to discard changes in working directory)"));
 	if (has_dirty_submodules)
 		status_printf_ln(s, c, _("  (commit or discard the untracked or modified content in submodules)"));
+	status_printf_ln(s, c, _("  Set advice.statusHints to false to turn off this message"));
 	status_printf_ln(s, c, "");
 }
 
@@ -225,6 +228,7 @@ static void wt_status_print_other_header(struct wt_status *s,
 	if (!advice_status_hints)
 		return;
 	status_printf_ln(s, c, _("  (use \"git %s <file>...\" to include in what will be committed)"), how);
+	status_printf_ln(s, c, _("  Set advice.statusHints to false to turn off this message"));
 	status_printf_ln(s, c, "");
 }
 
@@ -783,15 +787,23 @@ static void show_merge_in_progress(struct wt_status *s,
 {
 	if (has_unmerged(s)) {
 		status_printf_ln(s, color, _("You have unmerged paths."));
-		if (advice_status_hints)
+		if (advice_status_hints) {
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git commit\")"));
+			status_printf_ln(s, color,
+					 _("  Set advice.statusHints to"
+					   " false to turn off this message"));
+		}
 	} else {
 		status_printf_ln(s, color,
 			_("All conflicts fixed but you are still merging."));
-		if (advice_status_hints)
+		if (advice_status_hints) {
 			status_printf_ln(s, color,
 				_("  (use \"git commit\" to conclude merge)"));
+			status_printf_ln(s, color,
+					 _("  Set advice.statusHints to "
+					   "false to turn off this message"));
+		}
 	}
 	wt_status_print_trailer(s);
 }
@@ -813,6 +825,8 @@ static void show_am_in_progress(struct wt_status *s,
 			_("  (use \"git am --skip\" to skip this patch)"));
 		status_printf_ln(s, color,
 			_("  (use \"git am --abort\" to restore the original branch)"));
+		status_printf_ln(s, color,
+			 _("  Set advice.statusHints to false to turn off this message"));
 	}
 	wt_status_print_trailer(s);
 }
@@ -878,17 +892,25 @@ static void show_rebase_in_progress(struct wt_status *s,
 				_("  (use \"git rebase --skip\" to skip this patch)"));
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --abort\" to check out the original branch)"));
+			status_printf_ln(s, color,
+				_("  Set advice.statusHints to false to turn off this message"));
 		}
 	} else if (state->rebase_in_progress || !stat(git_path("MERGE_MSG"), &st)) {
 		status_printf_ln(s, color, _("You are currently rebasing."));
-		if (advice_status_hints)
+		if (advice_status_hints) {
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git rebase --continue\")"));
+			status_printf_ln(s, color,
+				_("  Set advice.statusHints to false to turn off this message"));
+		}
 	} else if (split_commit_in_progress(s)) {
 		status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
-		if (advice_status_hints)
+		if (advice_status_hints) {
 			status_printf_ln(s, color,
 				_("  (Once your working directory is clean, run \"git rebase --continue\")"));
+			status_printf_ln(s, color,
+				_("  Set advice.statusHints to false to turn off this message"));
+		}
 	} else {
 		status_printf_ln(s, color, _("You are currently editing a commit during a rebase."));
 		if (advice_status_hints && !s->amend) {
@@ -896,6 +918,8 @@ static void show_rebase_in_progress(struct wt_status *s,
 				_("  (use \"git commit --amend\" to amend the current commit)"));
 			status_printf_ln(s, color,
 				_("  (use \"git rebase --continue\" once you are satisfied with your changes)"));
+			status_printf_ln(s, color,
+				_("  Set advice.statusHints to false to turn off this message"));
 		}
 	}
 	wt_status_print_trailer(s);
@@ -913,6 +937,8 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 		else
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git commit\")"));
+		status_printf_ln(s, color,
+			_("  Set advice.statusHints to false to turn off this message"));
 	}
 	wt_status_print_trailer(s);
 }
@@ -922,9 +948,12 @@ static void show_bisect_in_progress(struct wt_status *s,
 				const char *color)
 {
 	status_printf_ln(s, color, _("You are currently bisecting."));
-	if (advice_status_hints)
+	if (advice_status_hints) {
 		status_printf_ln(s, color,
 			_("  (use \"git bisect reset\" to get back to the original branch)"));
+		status_printf_ln(s, color,
+			_("  Set advice.statusHints to false to turn off this message"));
+	}
 	wt_status_print_trailer(s);
 }
 
@@ -1011,10 +1040,16 @@ void wt_status_print(struct wt_status *s)
 		wt_status_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_files)
 			wt_status_print_other(s, &s->ignored, _("Ignored files"), "add -f");
-	} else if (s->commitable)
-		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),
-			advice_status_hints
-			? _(" (use -u option to show untracked files)") : "");
+	} else if (s->commitable) {
+		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed"));
+		if (advice_status_hints) {
+			status_printf_ln(s, GIT_COLOR_NORMAL,
+					 _(" (use -u option to show untracked files)"));
+			status_printf_ln(s, GIT_COLOR_NORMAL,
+					 _("  Set advice.statusHints to"
+					   " false to turn off this message"));
+		}
+	}
 
 	if (s->verbose)
 		wt_status_print_verbose(s);
@@ -1026,24 +1061,32 @@ void wt_status_print(struct wt_status *s)
 		else if (s->workdir_dirty) {
 			if (advice_status_hints)
 				printf(_("no changes added to commit "
-					 "(use \"git add\" and/or \"git commit -a\")\n"));
+					 "(use \"git add\" and/or \"git commit -a\")\n"
+					 "Set advice.statusHints to false"
+					 " to turn off this message\n"));
 			else
 				printf(_("no changes added to commit\n"));
 		} else if (s->untracked.nr) {
 			if (advice_status_hints)
 				printf(_("nothing added to commit but untracked files "
-					 "present (use \"git add\" to track)\n"));
+					 "present (use \"git add\" to track)\n"
+					 "Set advice.statusHints to false"
+					 " to turn off this message\n"));
 			else
 				printf(_("nothing added to commit but untracked files present\n"));
 		} else if (s->is_initial) {
 			if (advice_status_hints)
 				printf(_("nothing to commit (create/copy files "
-					 "and use \"git add\" to track)\n"));
+					 "and use \"git add\" to track)\n"
+					 "Set advice.statusHints to false"
+					 " to turn off this message\n"));
 			else
 				printf(_("nothing to commit\n"));
 		} else if (!s->show_untracked_files) {
 			if (advice_status_hints)
-				printf(_("nothing to commit (use -u to show untracked files)\n"));
+				printf(_("nothing to commit (use -u to show untracked files)\n"
+					 "Set advice.statusHints to false"
+					 " to turn off this message\n"));
 			else
 				printf(_("nothing to commit\n"));
 		} else
-- 8< --
-- 
Duy

Re: push.default documented in "man git-push"?

[Matthieu Moy]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> On Fri, Oct 05, 2012 at 01:03:28PM -0700, Junio C Hamano wrote:
>
>> OK, the messages are supposed to advise how to turn them off, so we
>> would want some code updates in that case.
>
> Something like this? It turns out none of the advice messages says
> anything about advice.*.

Personally, I'm fine with the current situation. Advanced users can read
the documentation, and for others, the messages are usually more helpful
than annoying.

> This patch makes many output more verbose/annoying,

For example, it makes the output of git status look like:

# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#   Set advice.statusHints to false to turn off this message
#
#       modified:   foo.txt
#
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#   Set advice.statusHints to false to turn off this message
#
#       modified:   foo.txt
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#   Set advice.statusHints to false to turn off this message
#
#       bar.txt

I think it's going really too far in verbosity, the actual information
is hidden by the advices.

> (which is good, more motivation to turn advice off).

I disagree. Having advices turned on doesn't harm anyone. I don't
remember anyone complaining about the verbosity of Git's advices. I've
seen *many* more people complaining about the user-unfriendliness of
Git.

I'm fine with very verbose (and scary) messages when the user did
something wrong (for example, the advice.implicitIdentity is something
you should never see if you configured Git properly before commiting).
But the user should not feel blamed for using the default configuration
of advice messages. The tripple repetition in "git status" shown above
really sounds like "hey, dumb user, why did you not set
advice.statusHints already", and is doubly scary for newbies, because
they are not told how to set the variable, nor what a config variable is
to Git.

I understand the motivation of making the advice.* section of "man
git-config" more easily discoverable, but then picking a reasonable
subset of the advice messages (e.g. places that are already verbose
anyway, and not places that total beginners would see on a daily basis
like "git status") would be much better than targetting exhaustivity
IMHO.

> @@ -89,7 +90,9 @@ void detach_advice(const char *new_name)
>  	"state without impacting any branches by performing another checkout.\n\n"
>  	"If you want to create a new branch to retain commits you create, you may\n"
>  	"do so (now or later) by using -b with the checkout command again. Example:\n\n"
> -	"  git checkout -b new_branch_name\n\n";
> +	"  git checkout -b new_branch_name\n"
> +	"Set either advice.detachedHead to false to turn off this message\n\n"
             ^^^^^^

Wrong cut-and paste from "... either XXX or YYY ..."? (repeated several
times below).

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

Re: push.default documented in "man git-push"?

[Nguyen Thai Ngoc Duy]

On Thu, Oct 11, 2012 at 9:18 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> For example, it makes the output of git status look like:
>
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #   Set advice.statusHints to false to turn off this message
> #
> #       modified:   foo.txt
> #
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #   Set advice.statusHints to false to turn off this message
> #
> #       modified:   foo.txt
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #   Set advice.statusHints to false to turn off this message
> #
> #       bar.txt

And two more lines on my output:

 no changes added to commit (use "git add" and/or "git commit -a")
 Set advice.statusHints to false to turn off this message

> I think it's going really too far in verbosity, the actual information
> is hidden by the advices.

We could make it only one extra line in this case by prepending "hint:
" before the advice and say "you could turn the hints off by setting
advice.statusHints" at the end. Not applicable in general though.

>> (which is good, more motivation to turn advice off).
>
> I disagree. Having advices turned on doesn't harm anyone. I don't
> remember anyone complaining about the verbosity of Git's advices. I've
> seen *many* more people complaining about the user-unfriendliness of
> Git.

"git status" is actually quite verbose, but the advice only plays a
part of it. So, not an actual complaint from me about the advice
alone.

>> -     "  git checkout -b new_branch_name\n\n";
>> +     "  git checkout -b new_branch_name\n"
>> +     "Set either advice.detachedHead to false to turn off this message\n\n"
>             ^^^^^^
>
> Wrong cut-and paste from "... either XXX or YYY ..."? (repeated several
> times below).

Cut&Paste mistake.
-- 
Duy

Re: push.default documented in "man git-push"?

[Junio C Hamano]

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

> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Fri, Oct 05, 2012 at 01:03:28PM -0700, Junio C Hamano wrote:
>>
>>> OK, the messages are supposed to advise how to turn them off, so we
>>> would want some code updates in that case.
>>
>> Something like this? It turns out none of the advice messages says
>> anything about advice.*.
>
> Personally, I'm fine with the current situation. Advanced users can read
> the documentation, and for others, the messages are usually more helpful
> than annoying.

OK, you convinced me.

> I disagree. Having advices turned on doesn't harm anyone. I don't
> remember anyone complaining about the verbosity of Git's advices. I've
> seen *many* more people complaining about the user-unfriendliness of
> Git.
>
> I'm fine with very verbose (and scary) messages when the user did
> something wrong (for example, the advice.implicitIdentity is something
> you should never see if you configured Git properly before commiting).
> But the user should not feel blamed for using the default configuration
> of advice messages. The tripple repetition in "git status" shown above
> really sounds like "hey, dumb user, why did you not set
> advice.statusHints already", and is doubly scary for newbies, because
> they are not told how to set the variable, nor what a config variable is
> to Git.

Let's let the list of advice.* messages in Documentation/config.txt
do their job.

Nguyen, thanks for taking a look.  I do agree the extra "here is how
to rob helpful hints from yourself before you learn them" seems to
do more harm than good.