[Sugar-devel] Proxy Settings in Network Control Panel

[Manuel Quiñones]

Hi Ajay and other devs,

Ajay, whlie reviewing your patch for the proxy feature, I decided to
step in and work on the issues I've found by myself.  I did this to
speed up the process, instead of going through a long
review/patch/review loop.  I hope you understand.  A (rather long,
sorry) description of what I did follows.  But first of all, I would
be glad if you can test my last patch:

https://bugs.sugarlabs.org/attachment/ticket/3070/0001-Add-proxy-configuration-support-to-Network-Control-P.patch

The first thing I've found is that your patch does both GConf and
GSettings.  I decided to go just for GSettings because Browse and all
underlayng GTK use it.  In any case I think you should have added
another mixin class for GSettings instead of adding code to GConfMixin
class.  Sasha's original patch had that quite separated to make adding
another backend an easy task.

So my first attempt was doing only GSettings and fixing the issues
I've found in yours:

https://bugs.sugarlabs.org/attachment/ticket/3070/proxy_gsettings.patch
(note this is not my final patch, let's call this my a) patch)

An issue with your patch: the UI does not always update when a setting
is changed.  TestCase: change a setting, quickly close the network
section and open it again: you will see the original value.  This is
because the timeout that calls _commit has not finished yet.  Sasha's
patch had gconf client.notify_add() for this.  In my a) patch I have
added a callback to the 'changed' event of gsettings that does the
same.

https://developer.gnome.org/gio/stable/GSettings.html#GSettings-changed

There are other differences between Sasha's patch and yours.  For
example you seem to forgot the try/finally clause in the _commit
method that blocks the __changed_cb callback.  I have added it back in
my a) patch.

I intentionally skipped $http_proxy environment variable (Sasha's patch 3/3):

http://lists.sugarlabs.org/archive/sugar-devel/2012-August/038804.html

This could be added later if there is a reason.  I don't see GNOME
doing it.  Which activities or parts of Sugar need it?  Indeed this
makes wget work with proxy by default but then why GNOME does not do
it?  Am I missing any conversation?

So, now for my final patch.  After having my a) patch working, I took
another path using the convenient way GSettings provides to bind
widgets and settings:

https://developer.gnome.org/gio/stable/GSettings.html#g-settings-bind

The adventages can be seen in the ~100 lines removed.  There is...

- no need for a mixin class,

- no need for connecting callbacks to signals between widgets and
settings in both directions

- no need to implement get_value_for_gsettings /
set_value_from_gsettings for each widget.  Sasha's gconf
implementation needed a big hierarchy of classes to archive this.

- no need to test the key type to map the corresponding getter ('s' to
use get_string(), for example, compare with my a) patch)

So, I welcome testing in different proxy scenarios for my patch.
Thanks a lot Ajay for your contributed work, which I took as basement.

-- 
.. manuq ..