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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 3, 2014, 21:35 UTC
Message-ID: <xmqqbnooq863.fsf@gitster.dls.corp.google.com>
In-Reply-To: <5457E7DF.5070500@web.de>

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

Show 22 quoted lines

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