On 03/01/2017 02:03 PM, Yuya Nishihara wrote:
On Tue, 28 Feb 2017 16:44:36 +0100, Pierre-Yves David wrote:
On 02/28/2017 03:29 PM, Yuya Nishihara wrote:
On Tue, 28 Feb 2017 11:35:03 +0100, Pierre-Yves David wrote:
On 02/28/2017 11:24 AM, Pierre-Yves David wrote:
On 02/28/2017 07:57 AM, Martin von Zweigbergk wrote:
On Mon, Feb 27, 2017 at 6:59 AM, Pierre-Yves David
<pierre-yves.da...@ens-lyon.org> wrote:
# HG changeset patch
# User Pierre-Yves David <pierre-yves.da...@ens-lyon.org>
# Date 1488044041 -3600
#      Sat Feb 25 18:34:01 2017 +0100
# Node ID c3224694bdae9cdb7530f952e2c767e419b7f280
# Parent  92526381242cd381375a465d5a800446916d2d7b
# EXP-Topic color
color: initialize color for local peer ui

The local peer

"local peer" or "localrepository"? The patch seems to be in "class
localrepository".

hum, good catch. Things seems clowner than I expected. It looks like we
don't use the "lui" (local ui, goes through uisetup) to create the
repository but the "baseui" (does not goes through uisetup).

Let me grab a shovel and go bad into that code.

Okay so in short "ui initialisation business is not simple". That
description should be:

color: initialize color for the localrepo ui

The 'ui' object dedicated to a 'localrepo' is independant from the one
available in dispatch (and 'uisetup'). In addition, it is created from
the 'baseui' (for good reason apparently). For this reason, we need to
run the color setup on it after the local repository config is read.

Maybe we can move color.setup() to ui.fixconfig(), where ui attributes are
updated reflecting to config changes.

I considered it but it did not seemed appropriate. Fixconfig is run
quite often and the setup function is erasing most internal state
related to color. I was not confident make that change. I'm not saying
it is entirely impossible but that requires a careful review of the
setup process and other fixconfig related thing. So I would rather not
go this route for now. In particular, running during 'fixconfig' would
mean running it 'too early' before the extensions are loaded (they might
change the default style.

Good point and agreed.

It would be nice if we can get rid of the color dependency from localrepo
in future.

There is two majors tasks in the way of having the color initialization more contained to the 'ui' class itself.

First, extensions can define a 'colortable' that augment the '_defaultstyles' mapping. This creates an order dependency between the initialization of extensions and the setup of color on the 'ui' object. That dependency makes things harder since we have to be careful to not initialize color too early. One easy way to remove this dependency is to split the style config in two layers. One "default" layer that would be "immutable" from a " ui" point of view (and updated by the extensions). And one extra layer at the ui level for change made through the configuration. When a style is requested, the local override would be searched before querying the global one. That was, update made by extensions to the global '_defaultstyle' could happen -after- the color is initialized on a 'ui' object.

Second, the current initialization of the color mode is meant as a one-time run and tend to modify, update and clear various mapping. It should me possible to update it to be a simple dispatch of value in a way that allow to setup the mode multiple time over the life of an 'ui' object. That should not be too hard but someone has to look into it.

I'm not planning to do that clean up for now since I've enough on my plate already. But I hope this message will hope someone clearing this in the future (maybe a me further in the future).

Cheers,

--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to