Lukas Renggli a écrit : Hi Lukas,

I have a couple of ideas below.

...

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

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?

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

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"

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

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. 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. 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)

...

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.

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

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).

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.

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

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 #()

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

...

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.

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.

Lukas

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

_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

Stéphane Ducasse a écrit :

...

...

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.

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