[PATCH] subrepos: add ui.recursesubrepos option

[Matt Mackall]

On Thu, 2011-09-22 at 16:14 +0200, Martin Geisler wrote:
> # HG changeset patch
> # User Martin Geisler <mg at aragost.com>
> # Date 1308244309 -7200
> # Node ID 558d09d8ff5b3ab6f812530e0e77c8b54aa397ec
> # Parent  91dc8878f88857f01b86a99ad047400704b5fd7e
> subrepos: add ui.recursesubrepos option
> 
> This option can be used to make commit, status, diff consistent with
> each other. If unset, the current behavior is kept unchanged:

>           | recurses   | recurses
>           | by default | with --subrepos
>   --------+------------+----------------
>   commit: | True       | n/a
>   status: | False      | True
>   diff:   | False      | True
> 
> If ui.recursesubrepos is set to a boolean B, then we have:
> 
>           | recurses   | recurses
>           | by default | with --subrepos
>   --------+------------+----------------
>   commit: | B          | True
>   status: | B          | True
>   diff:   | B          | True

I'm sorry, but this approach is still[1][2][3] not ok.

Here's the basic argument:

- tool calls status, gets a list of unknown files
- tool presents list of files to user
- user selects file
- tool attempts to call 'hg add' on file
- tool explodes because file was in a subrepo and hg add can't handle it

If status ever returns files by default that can't then be manipulated
by add/remove/forget, WE ARE BREAKING OUR API CONTRACT. I use "tool"
here instead of "user" as tools are much less forgiving about
contract-breakage, but this argument certainly applies to users as well.

Making it a config setting doesn't help (this was explicitly rejected in
March[1]). In general, we shouldn't put any major behavior changes under
config settings:

- experts getting a surprise when at a random user's keyboard is BAD
- a tool getting unexpected behavior because of a UI setting is BAD

It's ok for diff to ignore whitespace by default (minor); it's not ok
for pull to update by default (major). This is why we've done things
like deprecate [defaults] and add HGPLAIN. And disabling your option
with HGPLAIN will obviously not meet expectations of users who set that
option.

Again, the path forward is to add transparent subrepo support to
add/remove/forget/etc. first. Once that's done, we can talk about
changing the behavior of status. Similarly, diff recursing without a
switch is going to be blocked until import doesn't choke on its output.

Until then, I'm still going to stubbornly reject attempts to put the
cart before the horse.

[1] http://markmail.org/message/374pchj7wa5ajubf
[2] http://markmail.org/message/lhnplthvjt4xuupx
[3] http://markmail.org/message/5qbigdd6unxr7hus


-- 
Mathematics is the supreme nostalgia of our time.