[PATCH] submodule: Fix documentation of update subcommand

[PATCH] submodule: Fix documentation of update subcommand

[Michal Sojka]

The documentation says that submodule.$name.update can be overridden by
--checkout only if its value is `none`. This is not true, because both
implementation and documentation of --checkout specifies that the
override applies to all possible values.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 Documentation/git-submodule.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..84ab577 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -158,7 +158,7 @@ update::
 	checkout the commit specified in the index of the containing repository.
 	This will make the submodules HEAD be detached unless `--rebase` or
 	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
+	`rebase`, `merge` or `none`. This can be overridden by specifying
 	`--checkout`. Setting the key `submodule.$name.update` to `!command`
 	will cause `command` to be run. `command` can be any arbitrary shell
 	command that takes a single argument, namely the sha1 to update to.
-- 
2.1.1

Re: [PATCH] submodule: Fix documentation of update subcommand

[Junio C Hamano]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> The documentation says that submodule.$name.update can be overridden by
> --checkout only if its value is `none`. This is not true, because both
> implementation and documentation of --checkout specifies that the
> override applies to all possible values.
>
> Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
> ---
>  Documentation/git-submodule.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..84ab577 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -158,7 +158,7 @@ update::
>  	checkout the commit specified in the index of the containing repository.
>  	This will make the submodules HEAD be detached unless `--rebase` or
>  	`--merge` is specified or the key `submodule.$name.update` is set to
> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> +	`rebase`, `merge` or `none`. This can be overridden by specifying
>  	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>  	will cause `command` to be run. `command` can be any arbitrary shell
>  	command that takes a single argument, namely the sha1 to update to.

Thanks.  This looks sensible, judging only from the text (iow I
didn't check if there were legitimate reason why rebase/merge
settings should not be overriden from the command line).

Jens?

Re: [PATCH] submodule: Fix documentation of update subcommand

[Jens Lehmann]

Am 03.11.2014 um 20:02 schrieb Junio C Hamano:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> The documentation says that submodule.$name.update can be overridden by
>> --checkout only if its value is `none`. This is not true, because both
>> implementation and documentation of --checkout specifies that the
>> override applies to all possible values.
>>
>> Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
>> ---
>>   Documentation/git-submodule.txt | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 8e6af65..84ab577 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -158,7 +158,7 @@ update::
>>   	checkout the commit specified in the index of the containing repository.
>>   	This will make the submodules HEAD be detached unless `--rebase` or
>>   	`--merge` is specified or the key `submodule.$name.update` is set to
>> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
>> +	`rebase`, `merge` or `none`. This can be overridden by specifying
>>   	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>>   	will cause `command` to be run. `command` can be any arbitrary shell
>>   	command that takes a single argument, namely the sha1 to update to.
>
> Thanks.  This looks sensible, judging only from the text (iow I
> didn't check if there were legitimate reason why rebase/merge
> settings should not be overriden from the command line).

This was introduced in e6a1c43aaf (document submdule.$name.update=none
option for gitmodules), and I agree with Michal that we should fix it.
But I think we should rather say "This can be overridden by specifying
'--merge', '--rebase' or `--checkout`." here, as the other two options
also override the update setting. So I think we should queue this:

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..84ab577 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -158,7 +158,7 @@ update::
  	checkout the commit specified in the index of the containing repository.
  	This will make the submodules HEAD be detached unless `--rebase` or
  	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
+	`rebase`, `merge` or `none`. This can be overridden by using '--merge',
+	'--rebase' or
  	`--checkout`. Setting the key `submodule.$name.update` to `!command`
  	will cause `command` to be run. `command` can be any arbitrary shell
  	command that takes a single argument, namely the sha1 to update to.

Apart from that I'm all for it.

Re: [PATCH] submodule: Fix documentation of update subcommand

[Junio C Hamano]

I did a bit more digging of the history, and came up with this,
which would be with a clearer and fairer description.  Also to
clarify, I spelled what Michal's "This" meant to refer to.

-- >8 --
From: Michal Sojka <sojkam1@fel.cvut.cz>
Date: Mon, 3 Nov 2014 11:09:51 +0100
Subject: [PATCH] submodule: clarify documentation for update subcommand

e6a1c43a (document submdule.$name.update=none option for gitmodules,
2012-05-10) meant to say "Unlike the case where your .update
configuration is set to either 'rebase' or 'merge', when it is set
to 'none', the tip of the submodule would never move.  You can use
the --checkout option if you want the contents of the submodule to
be updated to some other commit."

But the resulting text made it sound as if using "--checkout" would
have no effect when .update configuration is set to 'rebase' or
'merge', which was misleading.  In fact, with the "--checkout"
option, the tip of the submodule moves to the exact commit that is
recorded in the superproject tree, regardless of .update
configuration.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-submodule.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..648323f 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -158,7 +158,7 @@ update::
 	checkout the commit specified in the index of the containing repository.
 	This will make the submodules HEAD be detached unless `--rebase` or
 	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
+	`rebase`, `merge` or `none`. The configuration can be overridden by specifying
 	`--checkout`. Setting the key `submodule.$name.update` to `!command`
 	will cause `command` to be run. `command` can be any arbitrary shell
 	command that takes a single argument, namely the sha1 to update to.
-- 
2.2.0-rc0-43-g5b91d12

Re: [PATCH] submodule: Fix documentation of update subcommand

[Jens Lehmann]

Am 03.11.2014 um 21:53 schrieb Junio C Hamano:
> I did a bit more digging of the history, and came up with this,
> which would be with a clearer and fairer description.  Also to
> clarify, I spelled what Michal's "This" meant to refer to.
>
> -- >8 --
> From: Michal Sojka <sojkam1@fel.cvut.cz>
> Date: Mon, 3 Nov 2014 11:09:51 +0100
> Subject: [PATCH] submodule: clarify documentation for update subcommand
>
> e6a1c43a (document submdule.$name.update=none option for gitmodules,
> 2012-05-10) meant to say "Unlike the case where your .update
> configuration is set to either 'rebase' or 'merge', when it is set
> to 'none', the tip of the submodule would never move.  You can use
> the --checkout option if you want the contents of the submodule to
> be updated to some other commit."
>
> But the resulting text made it sound as if using "--checkout" would
> have no effect when .update configuration is set to 'rebase' or
> 'merge', which was misleading.  In fact, with the "--checkout"
> option, the tip of the submodule moves to the exact commit that is
> recorded in the superproject tree, regardless of .update
> configuration.
>
> Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   Documentation/git-submodule.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..648323f 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -158,7 +158,7 @@ update::
>   	checkout the commit specified in the index of the containing repository.
>   	This will make the submodules HEAD be detached unless `--rebase` or
>   	`--merge` is specified or the key `submodule.$name.update` is set to
> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> +	`rebase`, `merge` or `none`. The configuration can be overridden by specifying
>   	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>   	will cause `command` to be run. `command` can be any arbitrary shell
>   	command that takes a single argument, namely the sha1 to update to.

Yup, but we should also mention '--merge' and '--rebase' here.

Re: [PATCH] submodule: Fix documentation of update subcommand

[Michal Sojka]

On Mon, Nov 03 2014, Junio C Hamano wrote:
> I did a bit more digging of the history, and came up with this,
> which would be with a clearer and fairer description.  Also to
> clarify, I spelled what Michal's "This" meant to refer to.
>
> -- >8 --
> From: Michal Sojka <sojkam1@fel.cvut.cz>
> Date: Mon, 3 Nov 2014 11:09:51 +0100
> Subject: [PATCH] submodule: clarify documentation for update subcommand
>
> e6a1c43a (document submdule.$name.update=none option for gitmodules,
> 2012-05-10) meant to say "Unlike the case where your .update
> configuration is set to either 'rebase' or 'merge', when it is set
> to 'none', the tip of the submodule would never move.  You can use
> the --checkout option if you want the contents of the submodule to
> be updated to some other commit."
>
> But the resulting text made it sound as if using "--checkout" would
> have no effect when .update configuration is set to 'rebase' or
> 'merge', which was misleading.  In fact, with the "--checkout"
> option, the tip of the submodule moves to the exact commit that is
> recorded in the superproject tree, regardless of .update
> configuration.

This is much better description than I was able to put together. Thanks.
I also agree with Jens that mentioning --merge and --rebase is
worthwhile.

-Michal

Re: [PATCH] submodule: Fix documentation of update subcommand

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> This was introduced in e6a1c43aaf (document submdule.$name.update=none
> option for gitmodules), and I agree with Michal that we should fix it.
> But I think we should rather say "This can be overridden by specifying
> '--merge', '--rebase' or `--checkout`." here, as the other two options
> also override the update setting. So I think we should queue this:
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..84ab577 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -158,7 +158,7 @@ update::
>  	checkout the commit specified in the index of the containing repository.
>  	This will make the submodules HEAD be detached unless `--rebase` or
>  	`--merge` is specified or the key `submodule.$name.update` is set to
> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> +	`rebase`, `merge` or `none`. This can be overridden by using '--merge',
> +	'--rebase' or
>  	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>  	will cause `command` to be run. `command` can be any arbitrary shell
>  	command that takes a single argument, namely the sha1 to update to.
>
> Apart from that I'm all for it.

But read the whole thing again.  Isn't that a bit roundabout and
tortuous?

The paragraph is about the "update" subcommand, and then mentions
how the subcommand is affected by options and configuration.  And
"OVERRIDING" the topic of this thread is only about configuration.

Disecting what each sentence in the existing paragraph says:

    - This is about updating the submodule working tree to match
      what the superproject expects.

    - There can be three ways how it is "updated" (and one way to
      leave it not updated), by setting submodule.$name.update
      and/or giving --rebase, --merge or --checkout option, and one
      way to leave it not "updated" by setting .update=none.

    - The .update=none can be defeated with --checkout

which I think is a mess.

It is a fairly common and uniform pattern that command line options
override configured defaults, so I think it could be argued that
"you can override .update=none or .update=anything with command line
option" is not even worth saying.  Definitely not by piling yet
another "oh by the way, if you have this, things behave differently
again" on top of existing description.

	Update the registered submodules to match what the superproject
	expects by cloning missing submodules and updating the
	working tree of the submodules.  The "updating" can take
	various forms:

	(1) By default, or by explicitly giving `--checkout` option,
            the HEAD of the submodules are detached to the exact
            commit recorded by the superproject.

	(2) By giving `--rebase` or `--merge` option, the commit
            that happens to be checked out in the submodule's
            working tree is integrated with the commit recorded by
            the superproject by rebasing or merging, respectively.

	Setting submodule.$name.update configuration to `rebase` or
        `merge` will make `git submodule update` without these
        command line options to default to `--rebase` or `--merge`,
        respectively.

	Also, setting submodule.$name.update configuration to `none`
        marks the named submodule not updated by "submodule update"
        by default (you can still use `--checkout`, `--merge`, or
        `--rebase`).

Or something perhaps?  Or the detailed description of
submodule.$name.update should be dropped from here and refer the
reader to config.txt instead?

Re: [PATCH] submodule: Fix documentation of update subcommand

[Michal Sojka]

On Mon, Nov 03 2014, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> This was introduced in e6a1c43aaf (document submdule.$name.update=none
>> option for gitmodules), and I agree with Michal that we should fix it.
>> But I think we should rather say "This can be overridden by specifying
>> '--merge', '--rebase' or `--checkout`." here, as the other two options
>> also override the update setting. So I think we should queue this:
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 8e6af65..84ab577 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -158,7 +158,7 @@ update::
>>  	checkout the commit specified in the index of the containing repository.
>>  	This will make the submodules HEAD be detached unless `--rebase` or
>>  	`--merge` is specified or the key `submodule.$name.update` is set to
>> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
>> +	`rebase`, `merge` or `none`. This can be overridden by using '--merge',
>> +	'--rebase' or
>>  	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>>  	will cause `command` to be run. `command` can be any arbitrary shell
>>  	command that takes a single argument, namely the sha1 to update to.
>>
>> Apart from that I'm all for it.
>
> But read the whole thing again.  Isn't that a bit roundabout and
> tortuous?
>
> The paragraph is about the "update" subcommand, and then mentions
> how the subcommand is affected by options and configuration.  And
> "OVERRIDING" the topic of this thread is only about configuration.
>
> Disecting what each sentence in the existing paragraph says:
>
>     - This is about updating the submodule working tree to match
>       what the superproject expects.
>
>     - There can be three ways how it is "updated" (and one way to
>       leave it not updated), by setting submodule.$name.update
>       and/or giving --rebase, --merge or --checkout option, and one
>       way to leave it not "updated" by setting .update=none.
>
>     - The .update=none can be defeated with --checkout
>
> which I think is a mess.
>
> It is a fairly common and uniform pattern that command line options
> override configured defaults, so I think it could be argued that
> "you can override .update=none or .update=anything with command line
> option" is not even worth saying.  Definitely not by piling yet
> another "oh by the way, if you have this, things behave differently
> again" on top of existing description.
>
> 	Update the registered submodules to match what the superproject
> 	expects by cloning missing submodules and updating the
> 	working tree of the submodules.  The "updating" can take
> 	various forms:
>
> 	(1) By default, or by explicitly giving `--checkout` option,
>             the HEAD of the submodules are detached to the exact
>             commit recorded by the superproject.
>
> 	(2) By giving `--rebase` or `--merge` option, the commit
>             that happens to be checked out in the submodule's
>             working tree is integrated with the commit recorded by
>             the superproject by rebasing or merging, respectively.
>
> 	Setting submodule.$name.update configuration to `rebase` or
>         `merge` will make `git submodule update` without these
>         command line options to default to `--rebase` or `--merge`,
>         respectively.
>
> 	Also, setting submodule.$name.update configuration to `none`
>         marks the named submodule not updated by "submodule update"
>         by default (you can still use `--checkout`, `--merge`, or
>         `--rebase`).

This sounds good, but it doesn't mention the `!command` value of
.update. I'd call this form (3). But then different update forms would
mix config settings and command line options.

> Or something perhaps?  Or the detailed description of
> submodule.$name.update should be dropped from here and refer the
> reader to config.txt instead?

I guess you mean gitmodules.txt.

The `!command` form is not documented in gitmodules.txt. Maybe it would
be best to fully document .update in gitmodules.txt and just refer to
there. Having documentation at two places seems to be confusing not only
for users, but also for those who send patches :)

I'm no longer able to formulate my proposal properly as a patch tonight,
but if needed I'll try it later.

-Michal

Re: [PATCH] submodule: Fix documentation of update subcommand

[Junio C Hamano]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> This sounds good, but it doesn't mention the `!command` value of
> .update.

That part is unchanged by what I did.  My rewrite was up to

	... by specifying `--checkout`.

of the existing text.

>> Or something perhaps?  Or the detailed description of
>> submodule.$name.update should be dropped from here and refer the
>> reader to config.txt instead?
>
> I guess you mean gitmodules.txt.

Actually, I do mean the configuration.  .gitmodules is just a
template to help the user populate .git/config, and the latter of
which should be the sole source of truth.  This is an important
principle, and it becomes even more important once we start talking
about security sensitive possiblity like allowing !command as the
value.

> The `!command` form is not documented in gitmodules.txt. Maybe it would
> be best to fully document .update in gitmodules.txt and just refer to
> there. Having documentation at two places seems to be confusing not only
> for users, but also for those who send patches :)
>
> I'm no longer able to formulate my proposal properly as a patch tonight,
> but if needed I'll try it later.

That is fine.  People have lived with the current text for more than
two years without problems, so we are obviously not in a hurry.

Re: [PATCH] submodule: Fix documentation of update subcommand

[Jens Lehmann]

Am 04.11.2014 um 00:08 schrieb Junio C Hamano:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>>> Or something perhaps?  Or the detailed description of
>>> submodule.$name.update should be dropped from here and refer the
>>> reader to config.txt instead?
>>
>> I guess you mean gitmodules.txt.
>
> Actually, I do mean the configuration.  .gitmodules is just a
> template to help the user populate .git/config, and the latter of
> which should be the sole source of truth.  This is an important
> principle, and it becomes even more important once we start talking
> about security sensitive possiblity like allowing !command as the
> value.

Not quite. You're definitely right about the !command value for
the 'update' setting; this should never be taken from .gitmodules
but only from .git/config. But apart from that following this
principle would hurt submodule users a lot. The only thing that
should be set in stone in .git/config is the 'url' setting,
because an older url might not even exist anmore. But e.g. the
'branch' setting must be taken from .gitmodules. Otherwise we
could not change it on a per-superproject-branch basis. And if
the 'path' setting would only be taken from .git/config instead
of .gitmodules, we wouldn't even be able to rename submodules
(which is exactly what this setting was added for in the first
place). The same applies to 'ignore' and 'fetch'.

So I believe that gitmodules.txt should describe all ćonfig
options that can be provided by upstream (and e.g. mention that
the 'url' and 'update' values are copied into .git/config on
init), while all settings that can be overridden locally should
be documented in config.txt (which will be a subset of those
documented in gitmodules.txt).

>> The `!command` form is not documented in gitmodules.txt. Maybe it would
>> be best to fully document .update in gitmodules.txt and just refer to
>> there. Having documentation at two places seems to be confusing not only
>> for users, but also for those who send patches :)
>>
>> I'm no longer able to formulate my proposal properly as a patch tonight,
>> but if needed I'll try it later.
>
> That is fine.  People have lived with the current text for more than
> two years without problems, so we are obviously not in a hurry.

Yup.

Re: [PATCH] submodule: Fix documentation of update subcommand

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> So I believe that gitmodules.txt should describe all ćonfig
> options that can be provided by upstream (and e.g. mention that
> the 'url' and 'update' values are copied into .git/config on
> init), while all settings that can be overridden locally should
> be documented in config.txt (which will be a subset of those
> documented in gitmodules.txt).

Right; thanks.

Re: [PATCH] submodule: Fix documentation of update subcommand

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Yup, but we should also mention '--merge' and '--rebase' here.

This has been sitting in the Stalled pile for quite a while and I am
getting tired of waiting.  How does this look?

-- >8 --
From: Michal Sojka <sojkam1@fel.cvut.cz>
Date: Mon, 3 Nov 2014 11:09:51 +0100
Subject: [PATCH] submodule: clarify documentation for update subcommand

e6a1c43a (document submdule.$name.update=none option for gitmodules,
2012-05-10) meant to say "Unlike the case where your .update
configuration is set to either 'rebase' or 'merge', when it is set
to 'none', the tip of the submodule would never move.  You can use
the --checkout option if you want the contents of the submodule to
be updated to some other commit."

But the resulting text made it sound as if using "--checkout" would
have no effect when .update configuration is set to 'rebase' or
'merge', which was misleading.  In fact, with the "--checkout"
option, the tip of the submodule moves to the exact commit that is
recorded in the superproject tree, regardless of .update
configuration.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-submodule.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..9bfcdf5 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -158,7 +158,8 @@ update::
 	checkout the commit specified in the index of the containing repository.
 	This will make the submodules HEAD be detached unless `--rebase` or
 	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
+	`rebase`, `merge` or `none`. The configuration can be overridden by
+	specifying `--rebase`, `--merge`, or
 	`--checkout`. Setting the key `submodule.$name.update` to `!command`
 	will cause `command` to be run. `command` can be any arbitrary shell
 	command that takes a single argument, namely the sha1 to update to.
-- 
2.3.0-301-g71e72fe

[PATCH v2] submodule: Fix documentation of update subcommand

[Michal Sojka]

The documentation of 'git submodule update' has several problems:

1) It says that submodule.$name.update can be overridden by --checkout
   only if its value is `none`. This is not true, because both
   implementation and documentation of --checkout specifies that the
   override applies to all possible values.

2) The documentation of submodule.$name.update key is scattered across
   three places, which is confusing.

3) The documentation of submodule.$name.update in gitmodules.txt is
   incorrect, because the code always uses the value from .git/config
   and never from .gitmodules.

This patch fixes all three problems. Now, submodule.$name.update is
fully documented in config.txt and the other files just refer to it.
This is based on discussion between myself, Junio C Hamano and Jens
Lehmann.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 Documentation/config.txt        | 27 ++++++++++++++++++++++-----
 Documentation/git-submodule.txt | 17 ++++++++---------
 Documentation/gitmodules.txt    | 18 ++++++------------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae6791d..f30cbbc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2411,12 +2411,29 @@ status.submodulesummary::
 
 submodule.<name>.path::
 submodule.<name>.url::
+	The path within this project and URL for a submodule. These
+	variables are initially populated by 'git submodule init';
+	edit them to override the URL and other values found in the
+	`.gitmodules` file. See linkgit:git-submodule[1] and
+	linkgit:gitmodules[5] for details.
+
 submodule.<name>.update::
-	The path within this project, URL, and the updating strategy
-	for a submodule.  These variables are initially populated
-	by 'git submodule init'; edit them to override the
-	URL and other values found in the `.gitmodules` file.  See
-	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
+	The default updating strategy for a submodule, used by `git
+	submodule update`. This variable is populated by `git
+	submodule init` from linkgit:gitmodules[5].
+
+	If the value is 'checkout' (the default), the new commit
+	specified in the superproject will be checked out in the
+	submodule on a detached HEAD.
+	If 'rebase', the current branch of the submodule will be
+	rebased onto the commit specified in the superproject.
+	If 'merge', the commit specified in the superproject will be
+	merged into the current branch in the submodule. If 'none',
+	the submodule with name `$name` will not be updated by
+	default.
+	If the value is of form '!command', it will cause `command` to
+	be run. `command` can be any arbitrary shell command that
+	takes a single argument, namely the sha1 to update to.
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..c92908e 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -154,14 +154,13 @@ If `--force` is specified, the submodule's work tree will be removed even if
 it contains local modifications.
 
 update::
-	Update the registered submodules, i.e. clone missing submodules and
-	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached unless `--rebase` or
-	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`. Setting the key `submodule.$name.update` to `!command`
-	will cause `command` to be run. `command` can be any arbitrary shell
-	command that takes a single argument, namely the sha1 to update to.
+	Update the registered submodules to match what the superproject
+	expects by cloning missing submodules and updating the working
+	tree of the submodules. The "updating" can take various forms
+	and can be configured in .git/config by the
+	`submodule.$name.update` key or by explicitely giving one of
+	'--checkout' (the default), '--merge' or '--rebase' options. See
+	linkgit:git-config[1] for details.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
@@ -302,7 +301,7 @@ the submodule itself.
 	Checkout the commit recorded in the superproject on a detached HEAD
 	in the submodule. This is the default behavior, the main use of
 	this option is to override `submodule.$name.update` when set to
-	`merge`, `rebase` or `none`.
+	other value than `checkout`.
 	If the key `submodule.$name.update` is either not explicitly set or
 	set to `checkout`, this option is implicit.
 
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f6c0dfd..59efbfe 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -38,18 +38,12 @@ submodule.<name>.url::
 In addition, there are a number of optional keys:
 
 submodule.<name>.update::
-	Defines what to do when the submodule is updated by the superproject.
-	If 'checkout' (the default), the new commit specified in the
-	superproject will be checked out in the submodule on a detached HEAD.
-	If 'rebase', the current branch of the submodule will be rebased onto
-	the commit specified in the superproject. If 'merge', the commit
-	specified in the superproject will be merged into the current branch
-	in the submodule.
-	If 'none', the submodule with name `$name` will not be updated
-	by default.
-
-	This config option is overridden if 'git submodule update' is given
-	the '--merge', '--rebase' or '--checkout' options.
+	Defines what to do when the submodule is updated by the
+	superproject. This is only used by `git submodule init` to
+	initialize the variable of the same name in .git/config.
+	Allowed values here are 'checkout', 'rebase', 'merge' or
+	'none'. See linkgit:git-config[1] for their meaning and other
+	values that can be configured manually by users.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.1.4

Re: [PATCH v2] submodule: Fix documentation of update subcommand

[Junio C Hamano]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> The documentation of 'git submodule update' has several problems:
>
> 1) It says that submodule.$name.update can be overridden by --checkout
>    only if its value is `none`.

Hmm, I do not read the existing sentence that way, though.  The
"only if" above is only in your head and not in the documentation,
no?  The way I understand it is that the explanation does not even
bother to say that it is overridable when update is set to something
that clearly corresponds to --option (e.g. 'update=rebase' is for
people too lazy to type --rebase from the command line), but because
it is unclear when it is set to 'update=none', it specifically
singles out that case.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ae6791d..f30cbbc 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2411,12 +2411,29 @@ status.submodulesummary::
>  
>  submodule.<name>.path::
>  submodule.<name>.url::
> +	The path within this project and URL for a submodule. These
> +	variables are initially populated by 'git submodule init';
> +	edit them to override the URL and other values found in the
> +	`.gitmodules` file. See linkgit:git-submodule[1] and
> +	linkgit:gitmodules[5] for details.
> +

OK.

>  submodule.<name>.update::
> -	The path within this project, URL, and the updating strategy
> -	for a submodule.  These variables are initially populated
> -	by 'git submodule init'; edit them to override the
> -	URL and other values found in the `.gitmodules` file.  See
> -	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
> +	The default updating strategy for a submodule, used by `git
> +	submodule update`. This variable is populated by `git
> +	submodule init` from linkgit:gitmodules[5].
> +
> +	If the value is 'checkout' (the default), the new commit
> +	specified in the superproject will be checked out in the

Have you formatted this?  I _think_ this change would break the
typesetting by having an empty line there.

> +	submodule on a detached HEAD.
> +	If 'rebase', the current branch of the submodule will be
> +	rebased onto the commit specified in the superproject.
> +	If 'merge', the commit specified in the superproject will be
> +	merged into the current branch in the submodule. If 'none',
> +	the submodule with name `$name` will not be updated by
> +	default.
> +	If the value is of form '!command', it will cause `command` to
> +	be run. `command` can be any arbitrary shell command that
> +	takes a single argument, namely the sha1 to update to.

I have a feeling that it is better to leave the explanations of
these values in git-submodule.txt (i.e. where you took the above
text from) and say "see description of 'update' command in
linkgit:git-submodule[1]" here to avoid duplication.

>  submodule.<name>.branch::
>  	The remote branch name for a submodule, used by `git submodule
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..c92908e 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -154,14 +154,13 @@ If `--force` is specified, the submodule's work tree will be removed even if
>  it contains local modifications.
>  
>  update::
> -	Update the registered submodules, i.e. clone missing submodules and
> -	checkout the commit specified in the index of the containing repository.
> -	This will make the submodules HEAD be detached unless `--rebase` or
> -	`--merge` is specified or the key `submodule.$name.update` is set to
> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> -	will cause `command` to be run. `command` can be any arbitrary shell
> -	command that takes a single argument, namely the sha1 to update to.
> +	Update the registered submodules to match what the superproject
> +	expects by cloning missing submodules and updating the working
> +	tree of the submodules....

This part is better than the original.

>  The "updating" can take various forms
> +	and can be configured in .git/config by the
> +	`submodule.$name.update` key or by explicitely giving one of
> +	'--checkout' (the default), '--merge' or '--rebase' options. See
> +	linkgit:git-config[1] for details.

Because submodule.<name>.update is interesting only to those who run
"git submodule update", and also the command line options that
interact with the setting are only described here not in config.txt,
I think it is better to have the description of various modes here.

And the description, if it is done here, can clarify the precedence
(i.e. command line trumps configuration) and semantics
(i.e. configuration 'update=checkout' and option --checkout are both
to trigger the same behaviour), perhaps like this:

	The updating can be done in one of three ways:

        checkout;; detaches the HEAD in the submodule at the commit
            that is recorded by the superproject.  This is done when
            --checkout option is given, or no option is given, and
            submodule.<name>.update is unset, or if it is set to
            'checkout'.
        rebase;; EXPLAIN IN A SIMILAR WAY, talk about --rebase,
            'rebase', etc.
        merge;; EXPLAIN IN A SIMILAR WAY, talk about --merge,
            'merge', etc.

        When no option is given and submodule.<name>.update is set
        to 'none', the submodule is not updated.

It would be awkward to talk about --option in any of the other pages
like config.txt and gitmodules.txt, but the relationship between the
options and configurations must be explained somewhere, so....

Re: [PATCH v2] submodule: Fix documentation of update subcommand

[Michal Sojka]

On Thu, Feb 19 2015, Junio C Hamano wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> The documentation of 'git submodule update' has several problems:
>>
>> 1) It says that submodule.$name.update can be overridden by --checkout
>>    only if its value is `none`.
>
> Hmm, I do not read the existing sentence that way, though.  The
> "only if" above is only in your head and not in the documentation,
> no?

Yes, you're right.

> The way I understand it is that the explanation does not even bother
> to say that it is overridable when update is set to something that
> clearly corresponds to --option (e.g. 'update=rebase' is for people
> too lazy to type --rebase from the command line), but because it is
> unclear when it is set to 'update=none', it specifically singles out
> that case.

I updated the commit message a bit.

>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index ae6791d..f30cbbc 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2411,12 +2411,29 @@ status.submodulesummary::
>>
>>  submodule.<name>.path::
>>  submodule.<name>.url::
>> +	The path within this project and URL for a submodule. These
>> +	variables are initially populated by 'git submodule init';
>> +	edit them to override the URL and other values found in the
>> +	`.gitmodules` file. See linkgit:git-submodule[1] and
>> +	linkgit:gitmodules[5] for details.
>> +
>
> OK.
>
>>  submodule.<name>.update::
>> -	The path within this project, URL, and the updating strategy
>> -	for a submodule.  These variables are initially populated
>> -	by 'git submodule init'; edit them to override the
>> -	URL and other values found in the `.gitmodules` file.  See
>> -	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
>> +	The default updating strategy for a submodule, used by `git
>> +	submodule update`. This variable is populated by `git
>> +	submodule init` from linkgit:gitmodules[5].
>> +
>> +	If the value is 'checkout' (the default), the new commit
>> +	specified in the superproject will be checked out in the
>
> Have you formatted this?  I _think_ this change would break the
> typesetting by having an empty line there.

Right. I need to add a '+' and deindent.

>> +	submodule on a detached HEAD.
>> +	If 'rebase', the current branch of the submodule will be
>> +	rebased onto the commit specified in the superproject.
>> +	If 'merge', the commit specified in the superproject will be
>> +	merged into the current branch in the submodule. If 'none',
>> +	the submodule with name `$name` will not be updated by
>> +	default.
>> +	If the value is of form '!command', it will cause `command` to
>> +	be run. `command` can be any arbitrary shell command that
>> +	takes a single argument, namely the sha1 to update to.
>
> I have a feeling that it is better to leave the explanations of
> these values in git-submodule.txt (i.e. where you took the above
> text from) and say "see description of 'update' command in
> linkgit:git-submodule[1]" here to avoid duplication.

OK

>>  submodule.<name>.branch::
>>  	The remote branch name for a submodule, used by `git submodule
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 8e6af65..c92908e 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -154,14 +154,13 @@ If `--force` is specified, the submodule's work tree will be removed even if
>>  it contains local modifications.
>>
>>  update::
>> -	Update the registered submodules, i.e. clone missing submodules and
>> -	checkout the commit specified in the index of the containing repository.
>> -	This will make the submodules HEAD be detached unless `--rebase` or
>> -	`--merge` is specified or the key `submodule.$name.update` is set to
>> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
>> -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>> -	will cause `command` to be run. `command` can be any arbitrary shell
>> -	command that takes a single argument, namely the sha1 to update to.
>> +	Update the registered submodules to match what the superproject
>> +	expects by cloning missing submodules and updating the working
>> +	tree of the submodules....
>
> This part is better than the original.

Indeed. You wrote this in a previous email :)

>>  The "updating" can take various forms
>> +	and can be configured in .git/config by the
>> +	`submodule.$name.update` key or by explicitely giving one of
>> +	'--checkout' (the default), '--merge' or '--rebase' options. See
>> +	linkgit:git-config[1] for details.
>
> Because submodule.<name>.update is interesting only to those who run
> "git submodule update", and also the command line options that
> interact with the setting are only described here not in config.txt,
> I think it is better to have the description of various modes here.
>
> And the description, if it is done here, can clarify the precedence
> (i.e. command line trumps configuration) and semantics
> (i.e. configuration 'update=checkout' and option --checkout are both
> to trigger the same behaviour), perhaps like this:
>
> 	The updating can be done in one of three ways:
>
>         checkout;; detaches the HEAD in the submodule at the commit
>             that is recorded by the superproject.  This is done when
>             --checkout option is given, or no option is given, and
>             submodule.<name>.update is unset, or if it is set to
>             'checkout'.
>         rebase;; EXPLAIN IN A SIMILAR WAY, talk about --rebase,
>             'rebase', etc.
>         merge;; EXPLAIN IN A SIMILAR WAY, talk about --merge,
>             'merge', etc.
>
>         When no option is given and submodule.<name>.update is set
>         to 'none', the submodule is not updated.
>
> It would be awkward to talk about --option in any of the other pages
> like config.txt and gitmodules.txt, but the relationship between the
> options and configurations must be explained somewhere, so....

Agreed expect that there is a fourth way: !command. But this could be
easily added here as well.

I'll send an updated patch in a while.

Thanks.
-Michal

[PATCH v3] submodule: Improve documentation of update subcommand

[Michal Sojka]

The documentation of 'git submodule update' has several problems:

1) It mentions that value 'none' of submodule.$name.update can be
   overridden by --checkout, but other combinations of configuration
   values and command line options are not mentioned.

2) The documentation of submodule.$name.update is scattered across three
   places, which is confusing.

3) The documentation of submodule.$name.update in gitmodules.txt is
   incorrect, because the code always uses the value from .git/config
   and never from .gitmodules.

4) Documentation of --force was incomplete, because it is only effective
   in case of checkout method of update.

This patch fixes all these problems. Now, submodule.$name.update is
fully documented in git-submodule.txt and the other files just refer to
it. This is based on discussion between Junio C Hamano, Jens Lehmann and
myself.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 Documentation/config.txt        | 15 +++++++----
 Documentation/git-submodule.txt | 58 +++++++++++++++++++++++++++++------------
 Documentation/gitmodules.txt    | 18 +++++--------
 3 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae6791d..fb2ae37 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2411,12 +2411,17 @@ status.submodulesummary::
 
 submodule.<name>.path::
 submodule.<name>.url::
+	The path within this project and URL for a submodule. These
+	variables are initially populated by 'git submodule init';
+	edit them to override the URL and other values found in the
+	`.gitmodules` file. See linkgit:git-submodule[1] and
+	linkgit:gitmodules[5] for details.
+
 submodule.<name>.update::
-	The path within this project, URL, and the updating strategy
-	for a submodule.  These variables are initially populated
-	by 'git submodule init'; edit them to override the
-	URL and other values found in the `.gitmodules` file.  See
-	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
+	The default updating strategy for a submodule. This variable
+	is populated by `git submodule init` from the
+	linkgit:gitmodules[5] file. See description of 'update'
+	command in linkgit:git-submodule[1].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..72c6fb2 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -154,14 +154,36 @@ If `--force` is specified, the submodule's work tree will be removed even if
 it contains local modifications.
 
 update::
-	Update the registered submodules, i.e. clone missing submodules and
-	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached unless `--rebase` or
-	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`. Setting the key `submodule.$name.update` to `!command`
-	will cause `command` to be run. `command` can be any arbitrary shell
-	command that takes a single argument, namely the sha1 to update to.
+	Update the registered submodules to match what the superproject
+	expects by cloning missing submodules and updating the working
+	tree of the submodules. The "updating" can be done in several
+	ways depending on command line options and the value of
+	`submodule.<name>.update` in .git/config:
+
+	checkout;; the new commit recorded in the superproject will be
+	    checked out in the submodule on a detached HEAD. This is
+	    done when `--checkout` option is given, or no option is
+	    given, and `submodule.<name>.update` is unset, or if it is set
+	    to 'checkout'.
+
+	rebase;; the current branch of the submodule will be rebased
+	    onto the commit recoded in the superproject. This is done
+	    when `--rebase` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'rebase'.
+
+	merge;; the commit recorded in the superproject will be merged
+	    into the current branch in the submodule. This is done
+	    when `--merge` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'merge'.
+
+	custom command;; arbitrary shell command that takes a single
+	    argument (the sha1 of the commit recorded in the
+	    superproject) is executed. This is done when no option is
+	    given, and `submodule.<name>.update` has the form of
+	    '!command'.
++
+When no option is given and `submodule.<name>.update` is set to 'none',
+the submodule is not updated.
 +
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
@@ -170,10 +192,11 @@ submodule with the `--init` option.
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
 +
-If `--force` is specified, the submodule will be checked out (using
-`git checkout --force` if appropriate), even if the commit specified in the
-index of the containing repository already matches the commit checked out in
-the submodule.
+If `--force` is specified and the checkout method of update is used, the
+submodule will be checked out (using `git checkout --force` if
+appropriate), even if the commit specified in the index of the
+containing repository already matches the commit checked out in the
+submodule.
 
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
@@ -238,10 +261,11 @@ OPTIONS
 	When running add, allow adding an otherwise ignored submodule path.
 	When running deinit the submodule work trees will be removed even if
 	they contain local changes.
-	When running update, throw away local changes in submodules when
-	switching to a different commit; and always run a checkout operation
-	in the submodule, even if the commit listed in the index of the
-	containing repository matches the commit checked out in the submodule.
+	When running update and the checkout method is used, throw away
+	local changes in submodules when switching to a different
+	commit; and always run a checkout operation in the submodule,
+	even if the commit listed in the index of the containing
+	repository matches the commit checked out in the submodule.
 
 --cached::
 	This option is only valid for status and summary commands.  These
@@ -302,7 +326,7 @@ the submodule itself.
 	Checkout the commit recorded in the superproject on a detached HEAD
 	in the submodule. This is the default behavior, the main use of
 	this option is to override `submodule.$name.update` when set to
-	`merge`, `rebase` or `none`.
+	other value than `checkout`.
 	If the key `submodule.$name.update` is either not explicitly set or
 	set to `checkout`, this option is implicit.
 
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f6c0dfd..a51183c 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -38,18 +38,12 @@ submodule.<name>.url::
 In addition, there are a number of optional keys:
 
 submodule.<name>.update::
-	Defines what to do when the submodule is updated by the superproject.
-	If 'checkout' (the default), the new commit specified in the
-	superproject will be checked out in the submodule on a detached HEAD.
-	If 'rebase', the current branch of the submodule will be rebased onto
-	the commit specified in the superproject. If 'merge', the commit
-	specified in the superproject will be merged into the current branch
-	in the submodule.
-	If 'none', the submodule with name `$name` will not be updated
-	by default.
-
-	This config option is overridden if 'git submodule update' is given
-	the '--merge', '--rebase' or '--checkout' options.
+	Defines what to do when the submodule is updated by the
+	superproject. This is only used by `git submodule init` to
+	initialize the variable of the same name in .git/config.
+	Allowed values here are 'checkout', 'rebase', 'merge' or
+	'none'. See description of 'update' command in
+	linkgit:git-submodule[1] for their meaning.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.1.4

Re: [PATCH v3] submodule: Improve documentation of update subcommand

[Junio C Hamano]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> This patch fixes all these problems. Now, submodule.$name.update is
> fully documented in git-submodule.txt and the other files just refer to

"Fix all these problems by documenting submodule.*.update in
git-submodule.txt and make everybody else refer to it" in imperative
mood, as if you are giving an order to the source to "be this way".
It would be sweeter and shorter that way.

> it. This is based on discussion between Junio C Hamano, Jens Lehmann and
> myself.

It's customary to just mention them on Helped-by: around here, I
think.

> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..72c6fb2 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -154,14 +154,36 @@ If `--force` is specified, the submodule's work tree will be removed even if
>  it contains local modifications.
>  
>  update::
> +	Update the registered submodules to match what the superproject
> +	expects by cloning missing submodules and updating the working
> +	tree of the submodules. The "updating" can be done in several
> +	ways depending on command line options and the value of
> +	`submodule.<name>.update` in .git/config:

No quoting around .git/config?  Actually, it is probably better not
to spell out that path.  "... and the value of the `...`
configuration variable" would be better.

> +	checkout;; the new commit recorded in the superproject will be
> +	    checked out in the submodule on a detached HEAD. This is

Drop "new".  It does not add anything to the description, and you
may even be checking out an old commit in the superproject.

> @@ -238,10 +261,11 @@ OPTIONS

Totally offtopic, but we may want a custom xfuncname for our AsciiDoc
documentation; we would want to see "--force::" not "OPTIONS" on the
above line, I would think.

>  	When running add, allow adding an otherwise ignored submodule path.
>  	When running deinit the submodule work trees will be removed even if
>  	they contain local changes.
> -	When running update, throw away local changes in submodules when
> -	switching to a different commit; and always run a checkout operation
> -	in the submodule, even if the commit listed in the index of the
> -	containing repository matches the commit checked out in the submodule.
> +	When running update and the checkout method is used, throw away
> +	local changes in submodules when switching to a different
> +	commit; and always run a checkout operation in the submodule,
> +	even if the commit listed in the index of the containing
> +	repository matches the commit checked out in the submodule.

This makes a reader wonder what --force would do when --merge or
--rebase is given from the command line (or specifiedy in the
configuration).  The original (unfortunately) did not have that
problem because it did not single out the --checkout mode.

The use of the phrase "the checkout method" is iffy, as nobody
defines what it is (I just said "--checkout mode" to mean the same
thing, but I do not think anybody defines it).  See below.


> @@ -302,7 +326,7 @@ the submodule itself.
>  	Checkout the commit recorded in the superproject on a detached HEAD
>  	in the submodule. This is the default behavior, the main use of
>  	this option is to override `submodule.$name.update` when set to
> -	`merge`, `rebase` or `none`.
> +	other value than `checkout`.

"... when set to a value other than `checkout`", would read better,
I would think.

> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index f6c0dfd..a51183c 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -38,18 +38,12 @@ submodule.<name>.url::
>  In addition, there are a number of optional keys:
>  
>  submodule.<name>.update::
> +	Defines what to do when the submodule is updated by the
> +	superproject. This is only used by `git submodule init` to
> +	initialize the variable of the same name in .git/config.
> +	Allowed values here are 'checkout', 'rebase', 'merge' or
> +	'none'. See description of 'update' command in
> +	linkgit:git-submodule[1] for their meaning.

Whatever word we decide to use, this may be a good place to
introduce it, perhaps like this (if we were to go with 'update
method'):

    submodule.<name>.update::

	Define the default update method for the named submodule,
	how the submodule is updated by "git submodule update"
	command in the superproject.

The enumeration of the allowed values is correct, I think, but we
might want to be very clear that we do not copy the !command form
and that is on purpose.

Re: [PATCH v3] submodule: Improve documentation of update subcommand

[Michal Sojka]

On Sat, Feb 21 2015, Junio C Hamano wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>>  update::
>> +	Update the registered submodules to match what the superproject
>> +	expects by cloning missing submodules and updating the working
>> +	tree of the submodules. The "updating" can be done in several
>> +	ways depending on command line options and the value of
>> +	`submodule.<name>.update` in .git/config:
>
> No quoting around .git/config?

There is not quoting in the rest of the file as well.

> Actually, it is probably better not to spell out that path. "... and
> the value of the `...` configuration variable" would be better.

Yes, I changed it to this. See the followup mail.

>>  	When running add, allow adding an otherwise ignored submodule path.
>>  	When running deinit the submodule work trees will be removed even if
>>  	they contain local changes.
>> -	When running update, throw away local changes in submodules when
>> -	switching to a different commit; and always run a checkout operation
>> -	in the submodule, even if the commit listed in the index of the
>> -	containing repository matches the commit checked out in the submodule.
>> +	When running update and the checkout method is used, throw away
>> +	local changes in submodules when switching to a different
>> +	commit; and always run a checkout operation in the submodule,
>> +	even if the commit listed in the index of the containing
>> +	repository matches the commit checked out in the submodule.
>
> This makes a reader wonder what --force would do when --merge or
> --rebase is given from the command line (or specifiedy in the
> configuration).  The original (unfortunately) did not have that
> problem because it did not single out the --checkout mode.

I changed that to "(only effective with the checkout method)".

> The use of the phrase "the checkout method" is iffy, as nobody
> defines what it is (I just said "--checkout mode" to mean the same
> thing, but I do not think anybody defines it).  See below.

I defined it in gitmodules.txt as you suggest as well as in the
description of update command in git-submodule.txt.

Thanks.
-Michal

[PATCH] submodule: Improve documentation of update subcommand

[Michal Sojka]

The documentation of 'git submodule update' has several problems:

1) It mentions that value 'none' of submodule.$name.update can be
   overridden by --checkout, but other combinations of configuration
   values and command line options are not mentioned.

2) The documentation of submodule.$name.update is scattered across three
   places, which is confusing.

3) The documentation of submodule.$name.update in gitmodules.txt is
   incorrect, because the code always uses the value from .git/config
   and never from .gitmodules.

4) Documentation of --force was incomplete, because it is only effective
   in case of checkout method of update.

Fix all these problems by documenting submodule.*.update in
git-submodule.txt and make everybody else refer to it.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 Documentation/config.txt        | 15 ++++++----
 Documentation/git-submodule.txt | 66 ++++++++++++++++++++++++++++-------------
 Documentation/gitmodules.txt    | 21 ++++++-------
 3 files changed, 65 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae6791d..fb2ae37 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2411,12 +2411,17 @@ status.submodulesummary::
 
 submodule.<name>.path::
 submodule.<name>.url::
+	The path within this project and URL for a submodule. These
+	variables are initially populated by 'git submodule init';
+	edit them to override the URL and other values found in the
+	`.gitmodules` file. See linkgit:git-submodule[1] and
+	linkgit:gitmodules[5] for details.
+
 submodule.<name>.update::
-	The path within this project, URL, and the updating strategy
-	for a submodule.  These variables are initially populated
-	by 'git submodule init'; edit them to override the
-	URL and other values found in the `.gitmodules` file.  See
-	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
+	The default updating strategy for a submodule. This variable
+	is populated by `git submodule init` from the
+	linkgit:gitmodules[5] file. See description of 'update'
+	command in linkgit:git-submodule[1].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..067d616 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -154,27 +154,51 @@ If `--force` is specified, the submodule's work tree will be removed even if
 it contains local modifications.
 
 update::
-	Update the registered submodules, i.e. clone missing submodules and
-	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached unless `--rebase` or
-	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`. Setting the key `submodule.$name.update` to `!command`
-	will cause `command` to be run. `command` can be any arbitrary shell
-	command that takes a single argument, namely the sha1 to update to.
 +
+--
+Update the registered submodules to match what the superproject
+expects by cloning missing submodules and updating the working tree of
+the submodules. The "updating" can be done in several ways depending
+on command line options and the value of `submodule.<name>.update`
+configuration variable. Supported update methods are:
+
+	checkout;; the commit recorded in the superproject will be
+	    checked out in the submodule on a detached HEAD. This is
+	    done when `--checkout` option is given, or no option is
+	    given, and `submodule.<name>.update` is unset, or if it is
+	    set to 'checkout'.
++
+If `--force` is specified, the submodule will be checked out (using
+`git checkout --force` if appropriate), even if the commit specified
+in the index of the containing repository already matches the commit
+checked out in the submodule.
+
+	rebase;; the current branch of the submodule will be rebased
+	    onto the commit recoded in the superproject. This is done
+	    when `--rebase` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'rebase'.
+
+	merge;; the commit recorded in the superproject will be merged
+	    into the current branch in the submodule. This is done
+	    when `--merge` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'merge'.
+
+	custom command;; arbitrary shell command that takes a single
+	    argument (the sha1 of the commit recorded in the
+	    superproject) is executed. This is done when no option is
+	    given, and `submodule.<name>.update` has the form of
+	    '!command'.
+
+When no option is given and `submodule.<name>.update` is set to 'none',
+the submodule is not updated.
+
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
-+
+
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
-+
-If `--force` is specified, the submodule will be checked out (using
-`git checkout --force` if appropriate), even if the commit specified in the
-index of the containing repository already matches the commit checked out in
-the submodule.
-
+--
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
@@ -238,10 +262,12 @@ OPTIONS
 	When running add, allow adding an otherwise ignored submodule path.
 	When running deinit the submodule work trees will be removed even if
 	they contain local changes.
-	When running update, throw away local changes in submodules when
-	switching to a different commit; and always run a checkout operation
-	in the submodule, even if the commit listed in the index of the
-	containing repository matches the commit checked out in the submodule.
+	When running update (only effective with the checkout method),
+	throw away local changes in submodules when switching to a
+	different commit; and always run a checkout operation in the
+	submodule, even if the commit listed in the index of the
+	containing repository matches the commit checked out in the
+	submodule.
 
 --cached::
 	This option is only valid for status and summary commands.  These
@@ -302,7 +328,7 @@ the submodule itself.
 	Checkout the commit recorded in the superproject on a detached HEAD
 	in the submodule. This is the default behavior, the main use of
 	this option is to override `submodule.$name.update` when set to
-	`merge`, `rebase` or `none`.
+	a value other than `checkout`.
 	If the key `submodule.$name.update` is either not explicitly set or
 	set to `checkout`, this option is implicit.
 
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f6c0dfd..7e8fb87 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -38,18 +38,15 @@ submodule.<name>.url::
 In addition, there are a number of optional keys:
 
 submodule.<name>.update::
-	Defines what to do when the submodule is updated by the superproject.
-	If 'checkout' (the default), the new commit specified in the
-	superproject will be checked out in the submodule on a detached HEAD.
-	If 'rebase', the current branch of the submodule will be rebased onto
-	the commit specified in the superproject. If 'merge', the commit
-	specified in the superproject will be merged into the current branch
-	in the submodule.
-	If 'none', the submodule with name `$name` will not be updated
-	by default.
-
-	This config option is overridden if 'git submodule update' is given
-	the '--merge', '--rebase' or '--checkout' options.
+	Defines the default update method for the named submodule,
+	i.e. how the submodule is updated by "git submodule update"
+	command in the superproject. This is only used by `git
+	submodule init` to initialize the configuration variable of
+	the same name. Allowed values here are 'checkout', 'rebase',
+	'merge' or 'none'. See description of 'update' command in
+	linkgit:git-submodule[1] for their meaning. Note that the
+	'!command' form is intentionally ignored here for security
+	reasons.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.1.4

Re: [PATCH] submodule: Improve documentation of update subcommand

[Junio C Hamano]

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> The documentation of 'git submodule update' has several problems:

Thanks, this round looks much better.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ae6791d..fb2ae37 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2411,12 +2411,17 @@ status.submodulesummary::
>  
>  submodule.<name>.path::
>  submodule.<name>.url::
> +	The path within this project and URL for a submodule. These
> +	variables are initially populated by 'git submodule init';
> +	edit them to override the URL and other values found in the
> +	`.gitmodules` file. See linkgit:git-submodule[1] and
> +	linkgit:gitmodules[5] for details.
> +

The sentence "edit them to override" talks about "other values",
which in the original wanted to cover not just "path" but "update"
as well.  By splitting 'update' into its own entry, "edit them to
override" is lost from 'update'.

But stepping back a bit, "edit them to override" applies to all
configuration variables.  The user edits the configuration file to
customize things.  I wonder if we even need to say this for .path
and url in the first place?

    Note: not a request to remove it because I hinted so, but a
    request for comments and discussion, as I do not have a firm
    opinion.

>  submodule.<name>.update::
> -	The path within this project, URL, and the updating strategy
> -	for a submodule.  These variables are initially populated
> -	by 'git submodule init'; edit them to override the
> -	URL and other values found in the `.gitmodules` file.  See
> -	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
> +	The default updating strategy for a submodule. This variable
> +	is populated by `git submodule init` from the
> +	linkgit:gitmodules[5] file. See description of 'update'
> +	command in linkgit:git-submodule[1].




> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 8e6af65..067d616 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -154,27 +154,51 @@ If `--force` is specified, the submodule's work tree will be removed even if
>  it contains local modifications.
>  
>  update::
> -	Update the registered submodules, i.e. clone missing submodules and
> -	checkout the commit specified in the index of the containing repository.
> -	This will make the submodules HEAD be detached unless `--rebase` or
> -	`--merge` is specified or the key `submodule.$name.update` is set to
> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
> -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
> -	will cause `command` to be run. `command` can be any arbitrary shell
> -	command that takes a single argument, namely the sha1 to update to.
>  +
> +--
> +Update the registered submodules to match what the superproject
> +expects by cloning missing submodules and updating the working tree of
> +the submodules. The "updating" can be done in several ways depending
> +on command line options and the value of `submodule.<name>.update`
> +configuration variable. Supported update methods are:

If you read the description of "--remote" (sorry, I didn't notice it
until I formatted the result of this patch and tried to read the
whole thing), we already use "update procedure" to mean these modes
of updates collectively.  Either use "update procedures" here (and
everywhere else in this patch where it is called "update method"),
or adjust the existing "update procedure" to "update method".
Either way is fine, but because "update procedure" is not wrong
per-se, I think it would be better to use that phrasing that may
already be familiar with the "git submodule" users.

Re: [PATCH] submodule: Improve documentation of update subcommand

[Junio C Hamano]

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

>> +Update the registered submodules to match what the superproject
>> +expects by cloning missing submodules and updating the working tree of
>> +the submodules. The "updating" can be done in several ways depending
>> +on command line options and the value of `submodule.<name>.update`
>> +configuration variable. Supported update methods are:
>
> If you read the description of "--remote" (sorry, I didn't notice it
> until I formatted the result of this patch and tried to read the
> whole thing), we already use "update procedure" to mean these modes
> of updates collectively.  Either use "update procedures" here (and
> everywhere else in this patch where it is called "update method"),
> or adjust the existing "update procedure" to "update method".
> Either way is fine, but because "update procedure" is not wrong
> per-se, I think it would be better to use that phrasing that may
> already be familiar with the "git submodule" users.

Addendum.  Your update to config.txt calls it "updating strategy".
That also needs to be unified to clarify that we are talking about
the same thing in these places to the readers.

Thanks.

Re: [PATCH] submodule: Improve documentation of update subcommand

[Michal Sojka]

On Mon, Feb 23 2015, Junio C Hamano wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
>
>> The documentation of 'git submodule update' has several problems:
>
> Thanks, this round looks much better.
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index ae6791d..fb2ae37 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2411,12 +2411,17 @@ status.submodulesummary::
>>  
>>  submodule.<name>.path::
>>  submodule.<name>.url::
>> +	The path within this project and URL for a submodule. These
>> +	variables are initially populated by 'git submodule init';
>> +	edit them to override the URL and other values found in the
>> +	`.gitmodules` file. See linkgit:git-submodule[1] and
>> +	linkgit:gitmodules[5] for details.
>> +
>
> The sentence "edit them to override" talks about "other values",
> which in the original wanted to cover not just "path" but "update"
> as well.  By splitting 'update' into its own entry, "edit them to
> override" is lost from 'update'.
>
> But stepping back a bit, "edit them to override" applies to all
> configuration variables.  The user edits the configuration file to
> customize things.  I wonder if we even need to say this for .path
> and url in the first place?
>
>     Note: not a request to remove it because I hinted so, but a
>     request for comments and discussion, as I do not have a firm
>     opinion.

I also thing that "edit them to override" is kind of useless here so I
removed it.

>
>>  submodule.<name>.update::
>> -	The path within this project, URL, and the updating strategy
>> -	for a submodule.  These variables are initially populated
>> -	by 'git submodule init'; edit them to override the
>> -	URL and other values found in the `.gitmodules` file.  See
>> -	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
>> +	The default updating strategy for a submodule. This variable
>> +	is populated by `git submodule init` from the
>> +	linkgit:gitmodules[5] file. See description of 'update'
>> +	command in linkgit:git-submodule[1].
>
>
>
>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 8e6af65..067d616 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -154,27 +154,51 @@ If `--force` is specified, the submodule's work tree will be removed even if
>>  it contains local modifications.
>>  
>>  update::
>> -	Update the registered submodules, i.e. clone missing submodules and
>> -	checkout the commit specified in the index of the containing repository.
>> -	This will make the submodules HEAD be detached unless `--rebase` or
>> -	`--merge` is specified or the key `submodule.$name.update` is set to
>> -	`rebase`, `merge` or `none`. `none` can be overridden by specifying
>> -	`--checkout`. Setting the key `submodule.$name.update` to `!command`
>> -	will cause `command` to be run. `command` can be any arbitrary shell
>> -	command that takes a single argument, namely the sha1 to update to.
>>  +
>> +--
>> +Update the registered submodules to match what the superproject
>> +expects by cloning missing submodules and updating the working tree of
>> +the submodules. The "updating" can be done in several ways depending
>> +on command line options and the value of `submodule.<name>.update`
>> +configuration variable. Supported update methods are:
>
> If you read the description of "--remote" (sorry, I didn't notice it
> until I formatted the result of this patch and tried to read the
> whole thing), we already use "update procedure" to mean these modes
> of updates collectively.  Either use "update procedures" here (and
> everywhere else in this patch where it is called "update method"),
> or adjust the existing "update procedure" to "update method".
> Either way is fine, but because "update procedure" is not wrong
> per-se, I think it would be better to use that phrasing that may
> already be familiar with the "git submodule" users.

OK, I replaced the method (and strategy in config.txt) with procedure.

In the previous version was also a typo, which is fixed now. v5 will
follow shortly.

Thanks.
-Michal

[PATCH v5] submodule: Improve documentation of update subcommand

[Michal Sojka]

The documentation of 'git submodule update' has several problems:

1) It mentions that value 'none' of submodule.$name.update can be
   overridden by --checkout, but other combinations of configuration
   values and command line options are not mentioned.

2) The documentation of submodule.$name.update is scattered across three
   places, which is confusing.

3) The documentation of submodule.$name.update in gitmodules.txt is
   incorrect, because the code always uses the value from .git/config
   and never from .gitmodules.

4) Documentation of --force was incomplete, because it is only effective
   in case of checkout method of update.

Fix all these problems by documenting submodule.*.update in
git-submodule.txt and make everybody else refer to it.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 Documentation/config.txt        | 15 ++++++----
 Documentation/git-submodule.txt | 66 ++++++++++++++++++++++++++++-------------
 Documentation/gitmodules.txt    | 21 ++++++-------
 3 files changed, 65 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae6791d..fb2ae37 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2411,12 +2411,17 @@ status.submodulesummary::
 
 submodule.<name>.path::
 submodule.<name>.url::
+	The path within this project and URL for a submodule. These
+	variables are initially populated by 'git submodule init';
+	edit them to override the URL and other values found in the
+	`.gitmodules` file. See linkgit:git-submodule[1] and
+	linkgit:gitmodules[5] for details.
+
 submodule.<name>.update::
-	The path within this project, URL, and the updating strategy
-	for a submodule.  These variables are initially populated
-	by 'git submodule init'; edit them to override the
-	URL and other values found in the `.gitmodules` file.  See
-	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
+	The default updating strategy for a submodule. This variable
+	is populated by `git submodule init` from the
+	linkgit:gitmodules[5] file. See description of 'update'
+	command in linkgit:git-submodule[1].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..067d616 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -154,27 +154,51 @@ If `--force` is specified, the submodule's work tree will be removed even if
 it contains local modifications.
 
 update::
-	Update the registered submodules, i.e. clone missing submodules and
-	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached unless `--rebase` or
-	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`. Setting the key `submodule.$name.update` to `!command`
-	will cause `command` to be run. `command` can be any arbitrary shell
-	command that takes a single argument, namely the sha1 to update to.
 +
+--
+Update the registered submodules to match what the superproject
+expects by cloning missing submodules and updating the working tree of
+the submodules. The "updating" can be done in several ways depending
+on command line options and the value of `submodule.<name>.update`
+configuration variable. Supported update methods are:
+
+	checkout;; the commit recorded in the superproject will be
+	    checked out in the submodule on a detached HEAD. This is
+	    done when `--checkout` option is given, or no option is
+	    given, and `submodule.<name>.update` is unset, or if it is
+	    set to 'checkout'.
++
+If `--force` is specified, the submodule will be checked out (using
+`git checkout --force` if appropriate), even if the commit specified
+in the index of the containing repository already matches the commit
+checked out in the submodule.
+
+	rebase;; the current branch of the submodule will be rebased
+	    onto the commit recoded in the superproject. This is done
+	    when `--rebase` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'rebase'.
+
+	merge;; the commit recorded in the superproject will be merged
+	    into the current branch in the submodule. This is done
+	    when `--merge` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'merge'.
+
+	custom command;; arbitrary shell command that takes a single
+	    argument (the sha1 of the commit recorded in the
+	    superproject) is executed. This is done when no option is
+	    given, and `submodule.<name>.update` has the form of
+	    '!command'.
+
+When no option is given and `submodule.<name>.update` is set to 'none',
+the submodule is not updated.
+
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
-+
+
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
-+
-If `--force` is specified, the submodule will be checked out (using
-`git checkout --force` if appropriate), even if the commit specified in the
-index of the containing repository already matches the commit checked out in
-the submodule.
-
+--
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
@@ -238,10 +262,12 @@ OPTIONS
 	When running add, allow adding an otherwise ignored submodule path.
 	When running deinit the submodule work trees will be removed even if
 	they contain local changes.
-	When running update, throw away local changes in submodules when
-	switching to a different commit; and always run a checkout operation
-	in the submodule, even if the commit listed in the index of the
-	containing repository matches the commit checked out in the submodule.
+	When running update (only effective with the checkout method),
+	throw away local changes in submodules when switching to a
+	different commit; and always run a checkout operation in the
+	submodule, even if the commit listed in the index of the
+	containing repository matches the commit checked out in the
+	submodule.
 
 --cached::
 	This option is only valid for status and summary commands.  These
@@ -302,7 +328,7 @@ the submodule itself.
 	Checkout the commit recorded in the superproject on a detached HEAD
 	in the submodule. This is the default behavior, the main use of
 	this option is to override `submodule.$name.update` when set to
-	`merge`, `rebase` or `none`.
+	a value other than `checkout`.
 	If the key `submodule.$name.update` is either not explicitly set or
 	set to `checkout`, this option is implicit.
 
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f6c0dfd..7e8fb87 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -38,18 +38,15 @@ submodule.<name>.url::
 In addition, there are a number of optional keys:
 
 submodule.<name>.update::
-	Defines what to do when the submodule is updated by the superproject.
-	If 'checkout' (the default), the new commit specified in the
-	superproject will be checked out in the submodule on a detached HEAD.
-	If 'rebase', the current branch of the submodule will be rebased onto
-	the commit specified in the superproject. If 'merge', the commit
-	specified in the superproject will be merged into the current branch
-	in the submodule.
-	If 'none', the submodule with name `$name` will not be updated
-	by default.
-
-	This config option is overridden if 'git submodule update' is given
-	the '--merge', '--rebase' or '--checkout' options.
+	Defines the default update method for the named submodule,
+	i.e. how the submodule is updated by "git submodule update"
+	command in the superproject. This is only used by `git
+	submodule init` to initialize the configuration variable of
+	the same name. Allowed values here are 'checkout', 'rebase',
+	'merge' or 'none'. See description of 'update' command in
+	linkgit:git-submodule[1] for their meaning. Note that the
+	'!command' form is intentionally ignored here for security
+	reasons.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.1.4

[PATCH v6] submodule: Improve documentation of update subcommand

[Michal Sojka]

The documentation of 'git submodule update' has several problems:

1) It mentions that value 'none' of submodule.$name.update can be
   overridden by --checkout, but other combinations of configuration
   values and command line options are not mentioned.

2) The documentation of submodule.$name.update is scattered across three
   places, which is confusing.

3) The documentation of submodule.$name.update in gitmodules.txt is
   incorrect, because the code always uses the value from .git/config
   and never from .gitmodules.

4) Documentation of --force was incomplete, because it is only effective
   in case of checkout method of update.

Fix all these problems by documenting submodule.*.update in
git-submodule.txt and make everybody else refer to it.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 Documentation/config.txt        | 14 +++++----
 Documentation/git-submodule.txt | 66 ++++++++++++++++++++++++++++-------------
 Documentation/gitmodules.txt    | 21 ++++++-------
 3 files changed, 64 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae6791d..0a6852d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2411,12 +2411,16 @@ status.submodulesummary::
 
 submodule.<name>.path::
 submodule.<name>.url::
+	The path within this project and URL for a submodule. These
+	variables are initially populated by 'git submodule init'. See
+	linkgit:git-submodule[1] and linkgit:gitmodules[5] for
+	details.
+
 submodule.<name>.update::
-	The path within this project, URL, and the updating strategy
-	for a submodule.  These variables are initially populated
-	by 'git submodule init'; edit them to override the
-	URL and other values found in the `.gitmodules` file.  See
-	linkgit:git-submodule[1] and linkgit:gitmodules[5] for details.
+	The default update procedure for a submodule. This variable
+	is populated by `git submodule init` from the
+	linkgit:gitmodules[5] file. See description of 'update'
+	command in linkgit:git-submodule[1].
 
 submodule.<name>.branch::
 	The remote branch name for a submodule, used by `git submodule
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..2c25916 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -154,27 +154,51 @@ If `--force` is specified, the submodule's work tree will be removed even if
 it contains local modifications.
 
 update::
-	Update the registered submodules, i.e. clone missing submodules and
-	checkout the commit specified in the index of the containing repository.
-	This will make the submodules HEAD be detached unless `--rebase` or
-	`--merge` is specified or the key `submodule.$name.update` is set to
-	`rebase`, `merge` or `none`. `none` can be overridden by specifying
-	`--checkout`. Setting the key `submodule.$name.update` to `!command`
-	will cause `command` to be run. `command` can be any arbitrary shell
-	command that takes a single argument, namely the sha1 to update to.
 +
+--
+Update the registered submodules to match what the superproject
+expects by cloning missing submodules and updating the working tree of
+the submodules. The "updating" can be done in several ways depending
+on command line options and the value of `submodule.<name>.update`
+configuration variable. Supported update procedures are:
+
+	checkout;; the commit recorded in the superproject will be
+	    checked out in the submodule on a detached HEAD. This is
+	    done when `--checkout` option is given, or no option is
+	    given, and `submodule.<name>.update` is unset, or if it is
+	    set to 'checkout'.
++
+If `--force` is specified, the submodule will be checked out (using
+`git checkout --force` if appropriate), even if the commit specified
+in the index of the containing repository already matches the commit
+checked out in the submodule.
+
+	rebase;; the current branch of the submodule will be rebased
+	    onto the commit recorded in the superproject. This is done
+	    when `--rebase` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'rebase'.
+
+	merge;; the commit recorded in the superproject will be merged
+	    into the current branch in the submodule. This is done
+	    when `--merge` option is given, or no option is given, and
+	    `submodule.<name>.update` is set to 'merge'.
+
+	custom command;; arbitrary shell command that takes a single
+	    argument (the sha1 of the commit recorded in the
+	    superproject) is executed. This is done when no option is
+	    given, and `submodule.<name>.update` has the form of
+	    '!command'.
+
+When no option is given and `submodule.<name>.update` is set to 'none',
+the submodule is not updated.
+
 If the submodule is not yet initialized, and you just want to use the
 setting as stored in .gitmodules, you can automatically initialize the
 submodule with the `--init` option.
-+
+
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and update any nested submodules within.
-+
-If `--force` is specified, the submodule will be checked out (using
-`git checkout --force` if appropriate), even if the commit specified in the
-index of the containing repository already matches the commit checked out in
-the submodule.
-
+--
 summary::
 	Show commit summary between the given commit (defaults to HEAD) and
 	working tree/index. For a submodule in question, a series of commits
@@ -238,10 +262,12 @@ OPTIONS
 	When running add, allow adding an otherwise ignored submodule path.
 	When running deinit the submodule work trees will be removed even if
 	they contain local changes.
-	When running update, throw away local changes in submodules when
-	switching to a different commit; and always run a checkout operation
-	in the submodule, even if the commit listed in the index of the
-	containing repository matches the commit checked out in the submodule.
+	When running update (only effective with the checkout procedure),
+	throw away local changes in submodules when switching to a
+	different commit; and always run a checkout operation in the
+	submodule, even if the commit listed in the index of the
+	containing repository matches the commit checked out in the
+	submodule.
 
 --cached::
 	This option is only valid for status and summary commands.  These
@@ -302,7 +328,7 @@ the submodule itself.
 	Checkout the commit recorded in the superproject on a detached HEAD
 	in the submodule. This is the default behavior, the main use of
 	this option is to override `submodule.$name.update` when set to
-	`merge`, `rebase` or `none`.
+	a value other than `checkout`.
 	If the key `submodule.$name.update` is either not explicitly set or
 	set to `checkout`, this option is implicit.
 
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index f6c0dfd..ac70eca 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -38,18 +38,15 @@ submodule.<name>.url::
 In addition, there are a number of optional keys:
 
 submodule.<name>.update::
-	Defines what to do when the submodule is updated by the superproject.
-	If 'checkout' (the default), the new commit specified in the
-	superproject will be checked out in the submodule on a detached HEAD.
-	If 'rebase', the current branch of the submodule will be rebased onto
-	the commit specified in the superproject. If 'merge', the commit
-	specified in the superproject will be merged into the current branch
-	in the submodule.
-	If 'none', the submodule with name `$name` will not be updated
-	by default.
-
-	This config option is overridden if 'git submodule update' is given
-	the '--merge', '--rebase' or '--checkout' options.
+	Defines the default update procedure for the named submodule,
+	i.e. how the submodule is updated by "git submodule update"
+	command in the superproject. This is only used by `git
+	submodule init` to initialize the configuration variable of
+	the same name. Allowed values here are 'checkout', 'rebase',
+	'merge' or 'none'. See description of 'update' command in
+	linkgit:git-submodule[1] for their meaning. Note that the
+	'!command' form is intentionally ignored here for security
+	reasons.
 
 submodule.<name>.branch::
 	A remote branch name for tracking updates in the upstream submodule.
-- 
2.1.4

Re: [PATCH v6] submodule: Improve documentation of update subcommand

[Junio C Hamano]

Thanks, will queue.  I think this round should be ready for 'next'.