Re: [PATCH] submodule: Improve 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:

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.