settings integration

Dear list,

I'm about to integrate some settings in replacement of some preferences,
and I would like to have your opinions about the way.

The new setting package make it possible to completely separate a
setting value
from the setting browser. In fact, the setting browser package should be
removeable
without any modification elsewhere.

As an example, let's  consider the #annotationPanes preference.
The only mandatory thing is that there is a class variable or an
instance variable of a singleton
in the package which owns the setting along with corresponding get/set
accessors.
For #annotationPanes, I define the #BrowsersHaveAnnotationPane class
variable in CodeHolder.
In addition, I define CodeHolder class>>browsersHaveAnnotationPane and
CodeHolder class>>browsersHaveAnnotationPane:.

In order to be able to manage this setting from the setting browser, we
have to define a setting declaration for it.
Because the setting browser must be removeable, I would  create a new
package for it.
Let's Tools-Base-Settings be this new package which then includes the
CodeHolderSettings class with the following declaration:

CodeHolderSettings class>>browsersHaveAnnotationPaneSetting
    <setting>
    ^ (SettingManager newSetting: 'Show annotation pane' translated)
parent: #codeBrowsingSettingNode;
            default: false;
            target: CodeHolder;
            setSelector: #browsersHaveAnnotationPane:;
            getSelector: #browsersHaveAnnotationPane;
            description: 'If checked then the annotation pane is shown
in browsers; it is dynamically updated with useful informations about
the code which is currently browsed' translated

Is it ok like that ?

Thanks
Alain







           


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Sounds excellent to me.
Imagine a removable Preference class.

Stef

On Nov 22, 2009, at 5:53 PM, Alain Plantec wrote:


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

I have a couple of ideas below.

1. The pragma <setting> seems to be a bit short and quite a common
name. Could be that others use this pragma already. Maybe something
slightly longer like <systemsettings> would be better?

2. To avoid having class references from everywhere onto the system
preference package I suggest that you pass into the method a builder
object (see configurations in Seaside 3.0). Like this there are no
dependencies and no compiler warnings. This would also allow to define
multiple preferences in one method.

3. getSelector:/setSelector: should be replaced by #selector:, the
write-accessor in most cases can be inferred from the read-accessor
(see Magritte). The target can be predefined to the receiving class,
that simplifies the definition.

4. Why is there a #default:? Normally a package should already have
initialized the setting, or does it lazily when the accessor is
performed the first time.

5. I don't know what #parent: means? If this is to group the settings
of a particular application I suggest to do that in the pragma.

After this "refactoring" I imagine that the code looks like this:

CodeHolder class>>systemSettings: aBuilder
    <systemsettings: 'Code Browser'>

    aBuilder newSetting
        label: 'Show annotation pane' translated;
        selector: #browsersHaveAnnotationPane;
        description: 'If checked then the annotation pane is shown ...'.
    aBuilder newSetting
        "defines more settings for that group"

Note, that I would put the code directly on the class side of
CodeHolder. I don't see a reason why CodeHolderSettings are required.
If the preferences are stored in class variables one can easily access
the settings.

Cheers,
Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Lukas Renggli a écrit :

Hi Lukas, ok, but note that you can use any other keyword. The default is actually
setting, it can be systemsetting.
you can also decide to use your own keyword for your project or your
application.
As an example, <seaside>, and then, browse them with
SystemSettingBrowser browse: 'seaside' "for seaside settings only"
or
SystemSettingBrowser browse: 'seaside|systemsetting' "for seaside and
systemsetting settings"
> 2. To avoid having class references from everywhere onto the system
> preference package I suggest that you pass into the method a builder
> object (see configurations in Seaside 3.0). Like this there are no
> dependencies and no compiler warnings. This would also allow to define
> multiple preferences in one method.
>  
I would'nt do that . I think it is better to have a specific package for
the settings.
Then, SettingManager is only referenced by packages which are dedicated
to settings management.
It makes them very easily removable.
see also my answer for you 'after refactoring' example.
> 3. getSelector:/setSelector: should be replaced by #selector:, the
> write-accessor in most cases can be inferred from the read-accessor
> (see Magritte). The target can be predefined to the receiving class,
> that simplifies the definition.
>  
done, but I would keep also getSelector:/setSelector:
> 4. Why is there a #default:? Normally a package should already have
> initialized the setting, or does it lazily when the accessor is
> performed the first time.
>  
the default is optional.
Do you think it is really a problem to keep it ?
> 5. I don't know what #parent: means? If this is to group the settings
> of a particular application I suggest to do that in the pragma.
>  
now we have a tree of settings (group allow only one level).
parent is the selector of the method which implements the parent setting
(the parent node in the tree).
I think it is better like this because :
- you can use sender/implementor
- when I implement a setting, I don't want to know/indicate the full path no, this is not like that because a setting declaration is never stored
(I mean in a global tree somewhere)
the tree is always dynamically built (no change management is to be
implemented).
- one method = one setting
- a setting method returns the setting declaration (which is never
stored but only used dynamically by the settings browser)

> Note, that I would put the code directly on the class side of
> CodeHolder. I don't see a reason why CodeHolderSettings are required.
> If the preferences are stored in class variables one can easily access
> the settings.
>  
Again one can decide to remove the system-settings package and then all
setting declarations.
So, I think is it better to have two packages.
Another reason is that a setting class variable could be defined only
for the need of the setting browser.
If it is declared in a setting specific package, then the system can
remain clean if the setting browser is removed.

thanks, thanks for your comments !!!

Cheers
Alain
> Cheers,
> Lukas
>
>  


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

I thought that there is a tree that already provides a kind of
grouping of settings. Why do you need another concept here? I assume
that one could just display all settings from the root, or starting at
an arbitrary node in the tree.

>> 2. To avoid having class references from everywhere onto the system
>> preference package I suggest that you pass into the method a builder
>> object (see configurations in Seaside 3.0). Like this there are no
>> dependencies and no compiler warnings. This would also allow to define
>> multiple preferences in one method.
>>
> I would'nt do that . I think it is better to have a specific package for
> the settings.

I don't want an extra package just with the settings for every package
I write. Also I don't want to have unreferenced classes in any of my
packages.

> Then, SettingManager is only referenced by packages which are dedicated
> to settings management.
> It makes them very easily removable.
> see also my answer for you 'after refactoring' example.

Still that introduces a lot of dependencies between packages. In your
example CodeHolder still depends on the preferences, because it
depends on the CodeHolder-Preferences that in turn depend on the
Preferences. Loading and unloading in the right order will be a
nightmare (without having the transcript full of undeclared warnings).

My suggestion completely decouples the setting declaration from the
setting framework. Metacello is following that pattern for its project
definitions with great success.

>> 3. getSelector:/setSelector: should be replaced by #selector:, the
>> write-accessor in most cases can be inferred from the read-accessor
>> (see Magritte). The target can be predefined to the receiving class,
>> that simplifies the definition.
>>
> done, but I would keep also getSelector:/setSelector:

Yes, definitely.

>> 4. Why is there a #default:? Normally a package should already have
>> initialized the setting, or does it lazily when the accessor is
>> performed the first time.
>>
> the default is optional.
> Do you think it is really a problem to keep it ?

Actually that was more of a question. Maybe specifying a default is
also useful to reset the settings to the default value?

>> 5. I don't know what #parent: means? If this is to group the settings
>> of a particular application I suggest to do that in the pragma.
>>
> now we have a tree of settings (group allow only one level).
> parent is the selector of the method which implements the parent setting
> (the parent node in the tree).
> I think it is better like this because :
> - you can use sender/implementor
> - when I implement a setting, I don't want to know/indicate the full path

Who defines that tree? Who defines the labels of this tree? Who
defines the icons of that tree?

> no, this is not like that because a setting declaration is never stored
> (I mean in a global tree somewhere)
> the tree is always dynamically built (no change management is to be
> implemented).

I imagine that the builder has some kind of a Seaside like API that
allows the methods to add and configure the settings, and probably
also group them into tree nodes. In the following example there is no
need to use the pragma for the groups (which is ugly indeed):

CodeHolder class>>settingsOn: aBuilder
   <settings>

   aBuilder group
      label: 'Code Browser';
      with: [
         aBuilder setting
         label: 'Annotation Pane';
         selector: #annotationPane.
      aBuilder setting
         label: 'Auto Format';
         selector: #autoFormat.
      aBuilder group
         label: 'Highlighting';
         with: [ self settingsHighlightOn: aBuilder ].
      aBuilder group
         label: 'Formatting';
         with: [ self settingsFormatOn: aBuilder ] ]

I think that would also make the whole thing much easier to understand.

> - one method = one setting

The configurable formatter of the refactoring browser has 23 settings.
Shout, the syntax highlighter, has 108 style definitions. It simply
does not scale to be forced to have to write a separate method for
each setting.

> - a setting method returns the setting declaration (which is never
> stored but only used dynamically by the settings browser)

I did not suggest to store the settings anywhere. The builder object
would be passed into all <settings> methods to build the tree
dynamically whenever needed.

Cheers,
Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Lukas Renggli a écrit :
>> SystemSettingBrowser browse: 'seaside|systemsetting' "for seaside and
>> systemsetting settings"
>>    
>
> I thought that there is a tree that already provides a kind of
> grouping of settings.
yes there is.
> Why do you need another concept here?
no new concept. It only makes use of a regexp instead of a string.
I only didn't want a "hardcoded" keyword (setting) and be able to change
it because I suspected that "setting" would not be ok for every one.
There is a default which is now "setting", which will be "systemsetting"
if we want and
"SystemSettingBrowser open" allows you to browse the default settings.

But imagine that someone do not want it's application setting to be
browsed together
with system settings. (debug, Free type, halo setting). It is simply
possible because the keywords
a setting browser is using can be passed as parameter.
> I assume
> that one could just display all settings from the root
yes
> , or starting at
> an arbitrary node in the tree.
>  
not for now but I can add it easily

> I don't want an extra package just with the settings for every package
> I write.
so, do as you want. If you want your settings to be declared together
with the rest of the code you simply can.
But it is interesting because this is mainly what I asked and I have an
answer. No, this is exactly what we want to avoid.
CodeHolder have its own class variable  and do not depend on any other
package regarding the setting management.
But there are some case where you can have to define some class variable
for the setting browsing only
(so only useful when the setting browser is available). I can give you
an example
if you want. Then, I think it is better to declare this class variable
in another package for better separation of concerns.
>  Loading and unloading in the right order will be a
> nightmare (without having the transcript full of undeclared warnings).
>  
no, you can load the Tool-Base package without having to load the
Tool-Base-Settings package.
Tool-Base-Settings is only loaded if you want the setting browser.
Imagine an image used headless. It does not need Tool-Base-Settings.
> My suggestion completely decouples the setting declaration from the
> setting framework. Metacello is following that pattern for its project
> definitions with great success.
>  
I like the builder solution. I find it very clean myself.
The problem is that I think we don't need it if we keep the setting
declaration as it is now.
Regarding your suggestion concerning the way settings are declared, then
yes a builder is a good solution.

>> the default is optional.
>> Do you think it is really a problem to keep it ?
>>    
>
> Actually that was more of a question. Maybe specifying a default is
> also useful to reset the settings to the default value?
>  
yes and also to know the kind of value if we have nil in the class
variable representing the setting.
But it is optional because one can also indicate the type explicitely in
a declaration.
>  
>>
>
> Who defines that tree?Who defines the labels of this tree? Who
> defines the icons of that tree?
>  
for a setting, all is given in a single setting declaration
(except the icon but good idea, I will add this possibility) At a first time, I liked that solution very much but now I don't.
If I understand well, if we use this pattern, then settings are not
discovered
on the fly anymore (and with it, I don't see the utility of the pragma
using except for root declarations).
the tree is hard coded is such #settingOn: methods and then you
introduce dependencies between settings.
>> - one method = one setting
>>    
>
> The configurable formatter of the refactoring browser has 23 settings.
> Shout, the syntax highlighter, has 108 style definitions. It simply
> does not scale to be forced to have to write a separate method for
> each setting.
you only do that one time thought.
but ok, I think I can easily allow several declarations in one method.
something like:

CodeHolderSetting class>>browserSettings
  <setting>
  ^ SettingManager newLevel parent: #userInterfaceSettings withAll: {
        SettingManager newSetting: 'Annotation Pane'; selector: #annotationPane.
        SettingManager newSetting: 'Auto format'; selector: #autoFormat.
        .... }

which is ok if you declare your settings in a specific package
or

CodeHolderSetting class>>browserSettingsOn: aManager
  <setting>
  ^ aManager newLevel parent: #userInterfaceSettings
        withAll: {
                aManager newSetting: 'Annotation Pane'; selector: #annotationPane.
                aManager newSetting: 'Auto format'; selector: #autoFormat. .... }

which is better if you do not want any setting specific package.

is it ok like that ?
if yes, the only point would be the #parent symbol.


> I did not suggest to store the settings anywhere. The builder object
> would be passed into all <settings> methods to build the tree
> dynamically whenever needed.
>  
ok, thank you very much for this design tips.
but as far as I understand it is not so useful here.


again, thank you for this discussion.

Cheers
Alain
> Cheers,
> Lukas
>
>  


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

> ok, thank you very much for this design tips.
> but as far as I understand it is not so useful here.
>  
re-tought a little bit:

CodeHolderSetting class>>browserSettingsOn: aManager
  <setting>
  ^ aManager newLevel parent: #userInterfaceSettings
        withAll: {
                aManager newSetting: 'Annotation Pane'; selector: #annotationPane.
                aManager newSetting: 'Auto format'; selector: #autoFormat. .... }

is the best solution.

Alain

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

> At a first time, I liked that solution very much but now I don't.
> If I understand well, if we use this pattern, then settings are not
> discovered
> on the fly anymore (and with it, I don't see the utility of the pragma
> using except for root declarations).
> the tree is hard coded is such #settingOn: methods and then you
> introduce dependencies between settings.

Sure, because you still want to be able to declare the settings
anywhere. And you want to be able to add settings methods as class
extensions to other classes. This is all possible, the only difference
is that the settings method has one argument and therefor no
dependency to anything whatsoever (which is absolutely crucial).

Yes, that would solve the problem of the 200 preferences. I still
prefer using a builder API, it would make it easier much easier to
read and build. Also consider that depending on { }-constructs for
settings declarations adds a dependency to Squeak/Pharo/GemStone,
because these array constructs are not part of of any language
specification.

Also consider that with a builder you can define the tree and the
settings at the same time. One package could define:

   " #groupNamed: creates a new group lazily or returns an existing
one, this allows to extend and reuse existing groups. #groupNamed:
does not depend on order, so the settings tree can be built in any
order. "
   (aBuilder groupNamed: #codeBrowser) with: [
       (aBuilder groupNamed: #highlight)
           label: 'Highlighting';
           description: 'The Shout Syntax Highlighter';
           with: [
              #(charLiteral stringLiteral numberLiteral blockArg
blockTemp ...) do: [ :e |
                 aBuilder setting ... ] ] ]

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Lukas Renggli a écrit : You convinced me on that point ok, but I still don't like having to express the entire absolute path.
What about this :

CodeHolderSetting class>>browserSettingsOn: aBuilder
 <setting>
        (aBuilder groupNamed: #highlight)
           parent: #codeBrowser; "create it lazily or get the existing one"
           label: 'Highlighting';
           description: 'The Shout Syntax Highlighter';
           with: [
              #(charLiteral stringLiteral ...blockTemp ...) do: [ :e |
                 aBuilder setting ... ] ]


then the two solutions can live together.
I will give it a try.
Thanks
Alain

> Lukas
>
>  


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

>
>
> Note, that I would put the code directly on the class side of
> CodeHolder. I don't see a reason why CodeHolderSettings are required.
> If the preferences are stored in class variables one can easily access
> the settings.

I imagine because we want to be able to remove any trace of preferences.
(may be could use class extension).

Stef

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

>
> Sure, because you still want to be able to declare the settings
> anywhere. And you want to be able to add settings methods as class
> extensions to other classes. This is all possible, the only difference
> is that the settings method has one argument and therefor no
> dependency to anything whatsoever (which is absolutely crucial).



CodeHolder class>>systemSettings: aBuilder
   <systemsettings: 'Code Browser'>

   aBuilder newSetting
       label: 'Show annotation pane' translated;
       selector: #browsersHaveAnnotationPane;
       description: 'If checked then the annotation pane is shown ...'.
   aBuilder newSetting
       "defines more settings for that group"

but lukas this is not because systemSetting:

        does not refer to setting classes that there is no dependency.
        newSetting  and all the rest will introduce a dependency.


> Yes, that would solve the problem of the 200 preferences. I still
> prefer using a builder API, it would make it easier much easier to
> read and build. Also consider that depending on { }-constructs for
> settings declarations adds a dependency to Squeak/Pharo/GemStone,
> because these array constructs are not part of of any language
> specification.

well....
at one point we should move.
Because we should ideally get rid of #()


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

>> I don't want an extra package just with the settings for every package
> I write. Also I don't want to have unreferenced classes in any of my
> packages.

But how can you then remove setting preference from your package?
Magic? but so far I do not see how.

        Package 1
                my app + class var for preferences (but no reference to preferences)

        Package 1-Pref
                reference to setting
       
        so that I can remove Package1-pref when we remove preferences.



>> Still that introduces a lot of dependencies between packages. In your
> example CodeHolder still depends on the preferences, because it
> depends on the CodeHolder-Preferences that in turn depend on the
> Preferences. Loading and unloading in the right order will be a
> nightmare (without having the transcript full of undeclared warnings).


No code Holder does not depend on Preference.
Now CodeHolder-Pref depend on Preference but just for the declaration of preference
not for its execution.
And Code-Holder Does not depend on Code-Holder-Pref this is the inverse


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Stéphane Ducasse a écrit : This is exactly what I would like to do for the migration from old
preferences to the new setting framework in the core.
But, as far as I understand, Lukas does'nt want to be constrained to
introduce a "Package 1-Pref" package.
This is why he prefers the builder pattern. Then, only at the price of
one argument, even if setting declarations are
implemented in "Package 1", no class dependency is introduced.
I still prefer the 2 packages solution, even with the builder pattern
because I think it is better to be able to
remove all setting declaration methods and eventually some class
variable if the setting package is removed.
But ok, let's go for this refactoring ! :)
Cheers
Alain



_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

> but lukas this is not because systemSetting:
>
>        does not refer to setting classes that there is no dependency.
>        newSetting  and all the rest will introduce a dependency.

Sure, that's a dependency too, but not one that prevents clean loading.

Hard dependencies in Smalltalk that prevent from loading are missing
super classes, missing classes that are extended, missing traits
imports, and missing class references. Pharo can somehow handle the
last dependency gracefully, but it shows up as an error in the
transcript and causes an undeclared variable to be defined in the
system. Other Smalltalk don't allow to load such code (VAST).

Personally I don't consider sending a non-existing message in a method
that is never called that much of a problem.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

On Nov 23, 2009, at 10:07 PM, Lukas Renggli wrote:

I see. So what I would do is to have a builder and also introduce most of the time
a separate package because this is really cleaner.


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project