The Inbox: System-ct.1119.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: System-ct.1119.mcz

commits-2
A new version of System was added to project The Inbox:
http://source.squeak.org/inbox/System-ct.1119.mcz

==================== Summary ====================

Name: System-ct.1119
Author: ct
Time: 26 October 2019, 10:00:59.779469 pm
UUID: dc499ab1-dfe1-064c-a997-271a591d4b18
Ancestors: System-mt.1116

Adds convenience methods to change multiple preferences

Usage example:

Preferences
        setPreferences: {
                #(UIManager >> #openToolsAttachedToMouseCursor) join asSymbol -> false.
                #(Model >> #useColorfulWindows) join asSymbol -> false }
        during: [self notify: 'Carpe Squeak!']

=============== Diff against System-mt.1116 ===============

Item was added:
+ ----- Method: Preferences class>>setPreferences: (in category 'get/set') -----
+ setPreferences: associations
+
+ associations associationsDo: [:association |
+ self setPreference: association key toValue: association value].!

Item was added:
+ ----- Method: Preferences class>>setPreferences:during: (in category 'get/set') -----
+ setPreferences: associations during: aBlock
+ "Changes the given values for the duration of aBlock"
+
+ | values previousValues |
+ values := associations as: Dictionary.
+ previousValues := values associations collect: [:association |
+ association key -> (self valueOfFlag: association key)].
+ self setPreferences: values.
+ ^ aBlock ensure: [
+ self setPreferences: previousValues]!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1119.mcz

marcel.taeumel
Hmm.... this sounds like it is meant to be used in tests only. Or are there other scenarios? Putting this in regular application code might yield confusing effects or hacky code.

So, what about putting this into an *SUnit extension category?

Best,
Marcel

Am 26.10.2019 22:01:13 schrieb [hidden email] <[hidden email]>:

A new version of System was added to project The Inbox:
http://source.squeak.org/inbox/System-ct.1119.mcz

==================== Summary ====================

Name: System-ct.1119
Author: ct
Time: 26 October 2019, 10:00:59.779469 pm
UUID: dc499ab1-dfe1-064c-a997-271a591d4b18
Ancestors: System-mt.1116

Adds convenience methods to change multiple preferences

Usage example:

Preferences
setPreferences: {
#(UIManager >> #openToolsAttachedToMouseCursor) join asSymbol -> false.
#(Model >> #useColorfulWindows) join asSymbol -> false }
during: [self notify: 'Carpe Squeak!']

=============== Diff against System-mt.1116 ===============

Item was added:
+ ----- Method: Preferences class>>setPreferences: (in category 'get/set') -----
+ setPreferences: associations
+
+ associations associationsDo: [:association |
+ self setPreference: association key toValue: association value].!

Item was added:
+ ----- Method: Preferences class>>setPreferences:during: (in category 'get/set') -----
+ setPreferences: associations during: aBlock
+ "Changes the given values for the duration of aBlock"
+
+ | values previousValues |
+ values := associations as: Dictionary.
+ previousValues := values associations collect: [:association |
+ association key -> (self valueOfFlag: association key)].
+ self setPreferences: values.
+ ^ aBlock ensure: [
+ self setPreferences: previousValues]!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1119.mcz

Christoph Thiede

Well, you could use it in methods such as Flaps class >> makeNavigatorFlapResembleGoldenBar or PreferenceWizardMorph >> toggleToolAndMenuIcons as well, but there are few of these senders outside of test methods.


However, I just discovered a possible duplicate, Preferences >> #setPreferencesFrom:. Do you think that's a problem? I think the old nested array style is less convenient compared to the use of associations, and there is no "during" version ...


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 29. Oktober 2019 10:59:30
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1119.mcz
 
Hmm.... this sounds like it is meant to be used in tests only. Or are there other scenarios? Putting this in regular application code might yield confusing effects or hacky code.

So, what about putting this into an *SUnit extension category?

Best,
Marcel

Am 26.10.2019 22:01:13 schrieb [hidden email] <[hidden email]>:

A new version of System was added to project The Inbox:
http://source.squeak.org/inbox/System-ct.1119.mcz

==================== Summary ====================

Name: System-ct.1119
Author: ct
Time: 26 October 2019, 10:00:59.779469 pm
UUID: dc499ab1-dfe1-064c-a997-271a591d4b18
Ancestors: System-mt.1116

Adds convenience methods to change multiple preferences

Usage example:

Preferences
setPreferences: {
#(UIManager >> #openToolsAttachedToMouseCursor) join asSymbol -> false.
#(Model >> #useColorfulWindows) join asSymbol -> false }
during: [self notify: 'Carpe Squeak!']

=============== Diff against System-mt.1116 ===============

Item was added:
+ ----- Method: Preferences class>>setPreferences: (in category 'get/set') -----
+ setPreferences: associations
+
+ associations associationsDo: [:association |
+ self setPreference: association key toValue: association value].!

Item was added:
+ ----- Method: Preferences class>>setPreferences:during: (in category 'get/set') -----
+ setPreferences: associations during: aBlock
+ "Changes the given values for the duration of aBlock"
+
+ | values previousValues |
+ values := associations as: Dictionary.
+ previousValues := values associations collect: [:association |
+ association key -> (self valueOfFlag: association key)].
+ self setPreferences: values.
+ ^ aBlock ensure: [
+ self setPreferences: previousValues]!




Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1119.mcz

marcel.taeumel
Hi Christoph,

you are proposing an interface to temporarily override specific preferences with an automatic restoring to their previous values after a work block. The examples you pointed to in Flaps and PreferenceWizardMorph would both not benefit from such a mechanism.

You are also proposing an interface to set preferences from a list of associations. Yes, this is kind of duplicate to the already existing #setPreferencesFrom:, which expects a list of pairs . Also, the "key" for pragma preferences (i.e., "Model>>#useColorfulWindows")  is kind of a private implementation detail, I suppose. I would rather not expose it that way.

Squeak's current mix of pragma preferences and "old-style" preferences is not optimal. I think we have the policy to add new preferences always as pragma preferencs.

I think that it is a good idea to add a mechanism to conveniently restore preferences after a work block. What about this kind of interface:

Preferences restoreAfter: [
   Browser showClassIcons: true.
   Preferences setFlag: #menuWithIcons toValue: switch.
   self doSomeThing].

It would be easy to just copy all the preferences and restore them after the workblock.

Still, I would rather not encourage such a trick in regular application code. Yet, it would be easy to hunt down and review all senders of #restoreAfter: from time to time ... Just my two cents. :-)

Best,
Marcel

Am 29.10.2019 12:47:55 schrieb Thiede, Christoph <[hidden email]>:

Well, you could use it in methods such as Flaps class >> makeNavigatorFlapResembleGoldenBar or PreferenceWizardMorph >> toggleToolAndMenuIcons as well, but there are few of these senders outside of test methods.


However, I just discovered a possible duplicate, Preferences >> #setPreferencesFrom:. Do you think that's a problem? I think the old nested array style is less convenient compared to the use of associations, and there is no "during" version ...


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 29. Oktober 2019 10:59:30
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1119.mcz
 
Hmm.... this sounds like it is meant to be used in tests only. Or are there other scenarios? Putting this in regular application code might yield confusing effects or hacky code.

So, what about putting this into an *SUnit extension category?

Best,
Marcel

Am 26.10.2019 22:01:13 schrieb [hidden email] <[hidden email]>:

A new version of System was added to project The Inbox:
http://source.squeak.org/inbox/System-ct.1119.mcz

==================== Summary ====================

Name: System-ct.1119
Author: ct
Time: 26 October 2019, 10:00:59.779469 pm
UUID: dc499ab1-dfe1-064c-a997-271a591d4b18
Ancestors: System-mt.1116

Adds convenience methods to change multiple preferences

Usage example:

Preferences
setPreferences: {
#(UIManager >> #openToolsAttachedToMouseCursor) join asSymbol -> false.
#(Model >> #useColorfulWindows) join asSymbol -> false }
during: [self notify: 'Carpe Squeak!']

=============== Diff against System-mt.1116 ===============

Item was added:
+ ----- Method: Preferences class>>setPreferences: (in category 'get/set') -----
+ setPreferences: associations
+
+ associations associationsDo: [:association |
+ self setPreference: association key toValue: association value].!

Item was added:
+ ----- Method: Preferences class>>setPreferences:during: (in category 'get/set') -----
+ setPreferences: associations during: aBlock
+ "Changes the given values for the duration of aBlock"
+
+ | values previousValues |
+ values := associations as: Dictionary.
+ previousValues := values associations collect: [:association |
+ association key -> (self valueOfFlag: association key)].
+ self setPreferences: values.
+ ^ aBlock ensure: [
+ self setPreferences: previousValues]!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1119.mcz

Christoph Thiede

Hi Marcel,


you are proposing an interface to temporarily override specific preferences with an automatic restoring to their previous values after a work block.


I think that's not actually a new interface, it's just a convenience method for #setPreference:toValue:during:. It should only complement #setPreferences: which was my actual proposal.


Do you think we should prefer the pair-based implementation (#setPreferencesFrom:) over the association-based proposal? If yes, I would like to complement it with a #setPreferencesFrom:during: implementation, just to align it with #setPreference:toValue: and #setPreference:toValue:during:.


Also, the "key" for pragma preferences (i.e., "Model>>#useColorfulWindows")  is kind of a private implementation detail, I suppose. I would rather not expose it that way.


Thank you for the pointer! I have often used this in test code and always ignored the fact that this reveals the private storage format of the Preferences class.
However, I find it quite useful to control all preferences from one central place. Should we maybe introduce an extra protocol for accessing pragma preferences?
Preferences setPreferenceFrom: Model key: #useColorfulWindows toValue: #false.
It feels so inconsistent to mix calls to Preferences with calls to separate classes ...

I would definitively favor something like
Preferences
    setPreferencesFrom: #(
        (balloonHelpEnabled false)
        (Model useColorfulWindows false)
        (TextEditor autoEnclose true))
    during: [self perfomTest]
-over-
Preferences setPreference: #balloonHelpEnabled toValue: false during: [
    lastModel := Model useColorfulWindows.
    enclose := TextEditor enclose.
    Model useColorfulWindows: false.
    TextEditor autoEnclose: true.
    [self performTest] ensure: [
        Model useColorfulWindows: lastModel.
        TextEditor autoEnclose: enclose]]!

(I'm a big fan of the execution around pattern :-))

Regarding your proposal #restoreAfter: - wouldn't this be an O(n) approach instead of an O(1) approach?


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 29. Oktober 2019 14:51:32
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1119.mcz
 
Hi Christoph,

you are proposing an interface to temporarily override specific preferences with an automatic restoring to their previous values after a work block. The examples you pointed to in Flaps and PreferenceWizardMorph would both not benefit from such a mechanism.

You are also proposing an interface to set preferences from a list of associations. Yes, this is kind of duplicate to the already existing #setPreferencesFrom:, which expects a list of pairs . Also, the "key" for pragma preferences (i.e., "Model>>#useColorfulWindows")  is kind of a private implementation detail, I suppose. I would rather not expose it that way.

Squeak's current mix of pragma preferences and "old-style" preferences is not optimal. I think we have the policy to add new preferences always as pragma preferencs.

I think that it is a good idea to add a mechanism to conveniently restore preferences after a work block. What about this kind of interface:

Preferences restoreAfter: [
   Browser showClassIcons: true.
   Preferences setFlag: #menuWithIcons toValue: switch.
   self doSomeThing].

It would be easy to just copy all the preferences and restore them after the workblock.

Still, I would rather not encourage such a trick in regular application code. Yet, it would be easy to hunt down and review all senders of #restoreAfter: from time to time ... Just my two cents. :-)

Best,
Marcel

Am 29.10.2019 12:47:55 schrieb Thiede, Christoph <[hidden email]>:

Well, you could use it in methods such as Flaps class >> makeNavigatorFlapResembleGoldenBar or PreferenceWizardMorph >> toggleToolAndMenuIcons as well, but there are few of these senders outside of test methods.


However, I just discovered a possible duplicate, Preferences >> #setPreferencesFrom:. Do you think that's a problem? I think the old nested array style is less convenient compared to the use of associations, and there is no "during" version ...


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 29. Oktober 2019 10:59:30
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1119.mcz
 
Hmm.... this sounds like it is meant to be used in tests only. Or are there other scenarios? Putting this in regular application code might yield confusing effects or hacky code.

So, what about putting this into an *SUnit extension category?

Best,
Marcel

Am 26.10.2019 22:01:13 schrieb [hidden email] <[hidden email]>:

A new version of System was added to project The Inbox:
http://source.squeak.org/inbox/System-ct.1119.mcz

==================== Summary ====================

Name: System-ct.1119
Author: ct
Time: 26 October 2019, 10:00:59.779469 pm
UUID: dc499ab1-dfe1-064c-a997-271a591d4b18
Ancestors: System-mt.1116

Adds convenience methods to change multiple preferences

Usage example:

Preferences
setPreferences: {
#(UIManager >> #openToolsAttachedToMouseCursor) join asSymbol -> false.
#(Model >> #useColorfulWindows) join asSymbol -> false }
during: [self notify: 'Carpe Squeak!']

=============== Diff against System-mt.1116 ===============

Item was added:
+ ----- Method: Preferences class>>setPreferences: (in category 'get/set') -----
+ setPreferences: associations
+
+ associations associationsDo: [:association |
+ self setPreference: association key toValue: association value].!

Item was added:
+ ----- Method: Preferences class>>setPreferences:during: (in category 'get/set') -----
+ setPreferences: associations during: aBlock
+ "Changes the given values for the duration of aBlock"
+
+ | values previousValues |
+ values := associations as: Dictionary.
+ previousValues := values associations collect: [:association |
+ association key -> (self valueOfFlag: association key)].
+ self setPreferences: values.
+ ^ aBlock ensure: [
+ self setPreferences: previousValues]!




Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-ct.1119.mcz

marcel.taeumel
Regarding your proposal #restoreAfter: - wouldn't this be an O(n) approach instead of an O(1) approach?

That depends on how it will be implemented. ;-) More imporantly, the user does not have to use special format for the pragma preferences. Just the usual code with a sandbox around it. That's convenient!

I would definitively favor something like ...  (Model useColorfulWindows false) ...

-1 too much disguise of a pragma preference. But that's just an opinion. :-) Also, preferences values can be complex objects such as colors or fill styles.

It feels so inconsistent to mix calls to Preferences with calls to separate classes ...

I know. Then again, it's just some global state where the usage should be minimized anyway. :-D

Best,
Marcel

Am 02.11.2019 03:05:29 schrieb Thiede, Christoph <[hidden email]>:

Hi Marcel,


you are proposing an interface to temporarily override specific preferences with an automatic restoring to their previous values after a work block.


I think that's not actually a new interface, it's just a convenience method for #setPreference:toValue:during:. It should only complement #setPreferences: which was my actual proposal.


Do you think we should prefer the pair-based implementation (#setPreferencesFrom:) over the association-based proposal? If yes, I would like to complement it with a #setPreferencesFrom:during: implementation, just to align it with #setPreference:toValue: and #setPreference:toValue:during:.


Also, the "key" for pragma preferences (i.e., "Model>>#useColorfulWindows")  is kind of a private implementation detail, I suppose. I would rather not expose it that way.


Thank you for the pointer! I have often used this in test code and always ignored the fact that this reveals the private storage format of the Preferences class.
However, I find it quite useful to control all preferences from one central place. Should we maybe introduce an extra protocol for accessing pragma preferences?
Preferences setPreferenceFrom: Model key: #useColorfulWindows toValue: #false.
It feels so inconsistent to mix calls to Preferences with calls to separate classes ...

I would definitively favor something like
Preferences
    setPreferencesFrom: #(
        (balloonHelpEnabled false)
        (Model useColorfulWindows false)
        (TextEditor autoEnclose true))
    during: [self perfomTest]
-over-
Preferences setPreference: #balloonHelpEnabled toValue: false during: [
    lastModel := Model useColorfulWindows.
    enclose := TextEditor enclose.
    Model useColorfulWindows: false.
    TextEditor autoEnclose: true.
    [self performTest] ensure: [
        Model useColorfulWindows: lastModel.
        TextEditor autoEnclose: enclose]]!

(I'm a big fan of the execution around pattern :-))

Regarding your proposal #restoreAfter: - wouldn't this be an O(n) approach instead of an O(1) approach?


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 29. Oktober 2019 14:51:32
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1119.mcz
 
Hi Christoph,

you are proposing an interface to temporarily override specific preferences with an automatic restoring to their previous values after a work block. The examples you pointed to in Flaps and PreferenceWizardMorph would both not benefit from such a mechanism.

You are also proposing an interface to set preferences from a list of associations. Yes, this is kind of duplicate to the already existing #setPreferencesFrom:, which expects a list of pairs . Also, the "key" for pragma preferences (i.e., "Model>>#useColorfulWindows")  is kind of a private implementation detail, I suppose. I would rather not expose it that way.

Squeak's current mix of pragma preferences and "old-style" preferences is not optimal. I think we have the policy to add new preferences always as pragma preferencs.

I think that it is a good idea to add a mechanism to conveniently restore preferences after a work block. What about this kind of interface:

Preferences restoreAfter: [
   Browser showClassIcons: true.
   Preferences setFlag: #menuWithIcons toValue: switch.
   self doSomeThing].

It would be easy to just copy all the preferences and restore them after the workblock.

Still, I would rather not encourage such a trick in regular application code. Yet, it would be easy to hunt down and review all senders of #restoreAfter: from time to time ... Just my two cents. :-)

Best,
Marcel

Am 29.10.2019 12:47:55 schrieb Thiede, Christoph <[hidden email]>:

Well, you could use it in methods such as Flaps class >> makeNavigatorFlapResembleGoldenBar or PreferenceWizardMorph >> toggleToolAndMenuIcons as well, but there are few of these senders outside of test methods.


However, I just discovered a possible duplicate, Preferences >> #setPreferencesFrom:. Do you think that's a problem? I think the old nested array style is less convenient compared to the use of associations, and there is no "during" version ...


Best,

Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 29. Oktober 2019 10:59:30
An: John Pfersich via Squeak-dev
Betreff: Re: [squeak-dev] The Inbox: System-ct.1119.mcz
 
Hmm.... this sounds like it is meant to be used in tests only. Or are there other scenarios? Putting this in regular application code might yield confusing effects or hacky code.

So, what about putting this into an *SUnit extension category?

Best,
Marcel

Am 26.10.2019 22:01:13 schrieb [hidden email] <[hidden email]>:

A new version of System was added to project The Inbox:
http://source.squeak.org/inbox/System-ct.1119.mcz

==================== Summary ====================

Name: System-ct.1119
Author: ct
Time: 26 October 2019, 10:00:59.779469 pm
UUID: dc499ab1-dfe1-064c-a997-271a591d4b18
Ancestors: System-mt.1116

Adds convenience methods to change multiple preferences

Usage example:

Preferences
setPreferences: {
#(UIManager >> #openToolsAttachedToMouseCursor) join asSymbol -> false.
#(Model >> #useColorfulWindows) join asSymbol -> false }
during: [self notify: 'Carpe Squeak!']

=============== Diff against System-mt.1116 ===============

Item was added:
+ ----- Method: Preferences class>>setPreferences: (in category 'get/set') -----
+ setPreferences: associations
+
+ associations associationsDo: [:association |
+ self setPreference: association key toValue: association value].!

Item was added:
+ ----- Method: Preferences class>>setPreferences:during: (in category 'get/set') -----
+ setPreferences: associations during: aBlock
+ "Changes the given values for the duration of aBlock"
+
+ | values previousValues |
+ values := associations as: Dictionary.
+ previousValues := values associations collect: [:association |
+ association key -> (self valueOfFlag: association key)].
+ self setPreferences: values.
+ ^ aBlock ensure: [
+ self setPreferences: previousValues]!