Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpPresenter class does not display instanceVariableNames: '' #7598

Closed
Ducasse opened this issue Oct 27, 2020 · 8 comments · Fixed by #7601
Closed

SpPresenter class does not display instanceVariableNames: '' #7598

Ducasse opened this issue Oct 27, 2020 · 8 comments · Fixed by #7601

Comments

@Ducasse
Copy link
Member

Ducasse commented Oct 27, 2020

SpPresenter class does not display instanceVariableNames: ''

This probably linked to the ClassPrinterDefinition now what is strange is that SpApplication shows it.

@MarcusDenker
Copy link
Member

maybe the reason is that SpAbstractPresenter (the superclass) uses a Trait?

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

I think that this is more related to abstractness of the class because the method

OldPharoClassDefinitionPrinter >> metaclassDefinitionString 

	^ String streamContents: [:stream |
		stream print: forClass.
		self halt.
		stream
			crtab;
			nextPutAll: 'instanceVariableNames: '''.
		forClass instanceVariablesOn: stream.
		stream nextPut: $' ]

is not invoked on SpPresenter.

I'm investigating.

ClyClassDefinitionEditorToolMorph >> buildTextMorph
	textModel := RubScrolledTextModel new.
	textModel interactionModel: self.
	textMorph := textModel newScrolledText.
	textMorph
		width: self width; "build is performed in background when owner is not exist yet. But we need proper width to perform kind of styling/formatting if needed"
		beWrapped;
		font: StandardFonts codeFont;
		editingMode: self editingMode.
	CmdKMDispatcher attachedTo: textMorph textArea withCommandsFrom: self. "It overrides default text morph shortcuts with Commander"
	self buildLeftSideBar.
	textModel setInitialText: self editingText.

I will put a break point in that method.

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

Well putting a halt in this method shows that it is called TWICE.

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

I'm totally confused by the logic of calypso :(

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

On

ClyClassDefinitionEditorToolMorph >> editingText
	
	^ editingClass definitionString

but probably before the template is displayed.

ClyClassCreationToolMorph >> editingText
	^self classTemplate 

And the template does not work with metaclass.

Why this is not simple :(

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

In P8 classTemplate was NOT handling metaclass correctly.

classTemplate

	| template |
	
	template := Slot showSlotClassDefinition
		ifTrue: [ 
			'Object subclass: #NameOfSubclass
	slots: {}
	classVariables: {}
	package: ''' ]
		ifFalse: [ 
			'Object subclass: #NameOfSubclass
	instanceVariableNames: ''''
	classVariableNames: ''''
	package: ''' ].

	^ template , self packageName , '''' 

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

So it means that a ClyClassCreationToolMorph is instantiated and another tool is then invoked.

In P8

ClyClassDefinitionEditorTool >> editingText
	self flag: #pharoTodo.
	^ editingClass definition

@Ducasse
Copy link
Member Author

Ducasse commented Oct 27, 2020

The bug is here

OldPharoSyntax >> traitedMetaclassDefinitionString 

	^ String streamContents: [:strm |
			strm print: forClass.
			forClass hasTraitComposition ifTrue: [
				strm
					crtab;
					nextPutAll: 'uses: ';
					print: forClass traitComposition ].
			forClass instanceVariablesString ifNotEmpty: [ 
				strm
					crtab;
					nextPutAll: 'instanceVariableNames: ';
					store: forClass instanceVariablesString ] ]

But I do not get why sometimes we get an empty string and sometimes not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants