[sugar] [review] controlpanel - modularize model

[Marco Pesenti Gritti]

On Sat, May 10, 2008 at 9:38 PM, Simon Schampijer <simon at schampijer.de> wrote:
> Hello,
>
> http://dev.laptop.org/~erikos/001_controlpanel.patch
> is the full patch.
>
> And you can see the splited patches (the last two) here:
> http://dev.laptop.org/git?p=users/erikos/sugar;a=shortlog;h=master

Very high level review, I'll not be able to do a detailed one until
I'm back from vacation... perhaps Tomeu can take this over.

-sys.path.insert(0, '@prefix@/share/sugar/shell')
+path = '@prefix@/share/sugar/shell'
+
+sys.path.insert(0, path)

I'd rather add shell_path to config.py.in

+src/controlpanel/icons/Makefile

We already setting up an icons path for the shell:

src/main.py:    icons_path = os.path.join(config.data_path, 'icons')

So I'd just put these icons in sugar/data/icons and install them in
data_path/icons

diff --git a/src/controlpanel/controltoolbar.py
b/src/controlpanel/controltoolbar.py

control in there sounds redundant, I'd just make it toolbar.py

+class DetailView(gtk.VBox):

DetailView sounds wrong to me. Maybe PanelView or PanelPage?

+    __gsignals__ = {
+        'valid-section': (gobject.SIGNAL_RUN_FIRST,
+                          gobject.TYPE_NONE,
+                          ([bool]))
+    }

Any reason this is not a property?

+class InlineAlert(gtk.EventBox):

Don't we have already an Alert widget in the toolkit? How is this different?

---

A couple of more general notes:

At some point we will start using gconf (or similars) and a python
model will be unnecessary for those preferences. Also they will
support change notification and the view will have to listen for
changes. It's probably fine for python models to *not* support change
notifications.

The undo stuff should be made more generic. A possible approach is to
split out the code which initialize the widgets to the current values
to a setup() method. Wrap the model module inside a Model object.
Model proxies the get_something/set_something method to the module,
and remembers the initial values. DetailView.undo() calls the
Model.undo()(which resets preferences to the inital values) and then
DetailView.setup().

Marco