>> 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"
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
[email protected]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
