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

[Junio C Hamano <gitster@pobox.com>]

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