pharo icon indicating copy to clipboard operation
pharo copied to clipboard

SettingTreeBuilder class >> acceptableKeywords: refers to unknown methods

Open Ducasse opened this issue 3 months ago • 8 comments

SettingTreeBuilder class >> acceptableKeywords: aCollectionOfRegExps
	^ self new acceptableKeywords: aCollectionOfRegExps

SettingTreeBuilder acceptableKeywords: { #notetakesettings } raises a DNU.

Ducasse avatar Nov 10 '25 20:11 Ducasse

In addition the API should be improved. Right now here is what the client should write

settings
	
	| builder |
	builder := SettingTreeBuilder new.
	(Pragma allNamed: #systemsettings in: self class) collect: [ :each | builder buildPragma: each ].
	^ builder nodeList

The SettingTreeBuilder class should encapsulate the building. I should be able to write something like

SettingTreeBuilder new on: self class buildFrom: #systemsettings

Again the writer could encapsulate the fact that it needs storedSetting. As a client why do I have to know.

storeSetting

	| writer |
	writer := SettingsStonWriter new.
	writer stream: ((FileLocator preferences / 'TheNoteTaker' / 'TNTSettings.config') asFileReference) writeStream.
	writer addSettings: (StoredSettingsFactory new fromSettingNodes: self settings).
	writer store.
	writer stream flush.
  • the flush shows that this not good because we force the client to know that he should flush.
  • Finally why a settings does not have a parent. Why do we need another object? Besides being forced to do it because of the MorphTreeMorph?

Ducasse avatar Nov 10 '25 20:11 Ducasse

I should split this issue in two but for now let us make sure it contains information because it took more than I wanted to get it running.

Ducasse avatar Nov 10 '25 20:11 Ducasse

How should we read and understand this code

storedValueForSettingNode: aSettingNode
	"It returns a stored value for the setting node.
	It returns nil value when there is no stored value."
	^ (self storedSettingForSettingNode: aSettingNode) ifNotNil: #realValue

Probably it means

storedValueForSettingNode: aSettingNode
	"It returns a stored value for the setting node.
	It returns nil value when there is no stored value."
	^ (self storedSettingForSettingNode: aSettingNode) ifNotNil: [ :s | s realValue ]
  • Why do we force the reader to learn that certain methods do have optional argument?

Ducasse avatar Nov 10 '25 21:11 Ducasse

This is how we go from the node to the setting declaration.

SettingNode >> realValue: anObject
	
	self settingDeclaration realValue: anObject

and where the SettingDeclaration actually changes the value of the object representing the setting (for example a classVariable).

Ducasse avatar Nov 11 '25 19:11 Ducasse

Here is an example for settingNodeIdentifier. Apparently this is path in the tree

[
	StoredSetting {
		#settingNodeIdentifier : '#tnt_note#GitEnable',
		#realValue : false
	},
	AbsolutePathStoredSetting {
		#settingNodeIdentifier : '#tnt_note#CurrentFolder',
		#realValue : [
			'Users',
			'XXXX',
			'Documents',
			'noteTaker'
		]
	}
]

Ducasse avatar Nov 11 '25 19:11 Ducasse

SettingsTree is using the SettingTreeBuilder to get its nodes.

SettingsTree >> nodeList
	| builder |
	^ nodeList
		ifNil: [
			builder := SettingTreeBuilder new.
			self pragmasDo: [:p | builder buildPragma: p].
			nodeList := builder nodeList.
			nodeList do: [:n | n model: self].
			self checkForUnknownParent.
			self checkForUnknownTarget.
			self nodeList ]

Ducasse avatar Nov 11 '25 20:11 Ducasse

(SettingTree acceptableKeywords: #(#'systemsettings')) settingTreeRoots.

Ducasse avatar Nov 11 '25 20:11 Ducasse

I want to be able to use my own pragma settings. I could invoke it that way

(SettingTree acceptableKeywords: #(#notetakersettings)) settingTreeRoots.

Why buildPragma: aPragma is not implemented as

SettingTreeBuilder >> buildPragma: aPragma
	currentPragma := aPragma.
	aPragma method methodClass instanceSide
		perform: aPragma method selector 
		with: self.
	^ nodeList

Because the current implementation hereafter implies that the treebuilder has a method systemSetting (because it performs the pragma selector!

SettingTreeBuilder >> buildPragma: aPragma
	currentPragma := aPragma.
	self perform: aPragma selector withArguments: aPragma arguments.
	^ nodeList

Especially since systemsettings is implemented as

SettingTreeBuilder >> systemsettings
	"Process a <systemsettings> pragma"

	<settingPragmaProcessor>
	currentPragma methodClass instanceSide
		perform: currentPragma methodSelector
		with: self
```		

The tag  <settingPragmaProcessor> is so that the  systemsettings is recognized as systemsetting.

settingsKeywords ^ (Pragma allNamed: #settingPragmaProcessor) collect: [:p | p method selector]


it means that we can have a specific setting tag. 

We just need to tag a method named with the name of the pragma that identify the new setting e.g. `notetakersettings` with `<settingPragmaProcessor>` to get a <notetakersettings>


	

Ducasse avatar Nov 11 '25 20:11 Ducasse