subrepos: recursive status/diff with ui.commitsubrepos=True

[Matt Mackall]

On Wed, 2011-11-30 at 12:51 +0100, Martin Geisler wrote:
> Hi everybody,
> 
> We recently changed the default for ui.commitsubrepos to False. The
> behavior of commit, status, and diff now look like this:
> 
>           | recurses   | recurses        | recurses with
>           | by default | with --subrepos | ui.commitsubrepos=B
>   --------+------------+---------------- +--------------------
>   commit: | False      | True            | B
>   status: | False      | True            | False
>   diff:   | False      | True            | False
> 
> I've added a new, third column to show how the ui.commitsubrepos option
> impact the three commands.

I think your table is extremely misleading, indeed it has misled me
before. Let's rewrite it:

        (default)
        commitsubrepos=False  commitsubrepos=True  with --subrepos
commit  no recurse / abort    recurse              recurse
status  no recurse            no recurse           recurse
diff    no recurse            no recurse           recurse

> The table is almost consistent..., except for the third column. Can we
> make it consistent by putting B's all the down in that column?

I think the current table's fine. The center column shouldn't be changed
(what you're proposing) because it's what people have to use to get the
legacy behavior but we really just want to pretend this column doesn't
exist moving forward. The third column is obviously correct and is not
particularly worth talking about. The only interesting place is column
one.

> When I proposed the ui.recursesubrepos option I had B's all the way down
> in the last columns. The objection was that tools would be confused. I
> tried to argue that HGPLAIN should turn the new behavior off. I don't
> think this was countered further and I believe that HGPLAIN should make
> the change "tool-safe" until full transparency is achieved.

It certainly was countered!

        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.

http://markmail.org/message/arfyxwz6yetzihzw

Do you really want to put "RANDOM TOOLS WILL COMPLETELY IGNORE THIS
SETTING, ENJOY THE RESULTING CONFUSION COURTESY OF MARTIN GEISLER" in
the docs for your proposed option?

-- 
Mathematics is the supreme nostalgia of our time.