iceberg icon indicating copy to clipboard operation
iceberg copied to clipboard

History browser install action does not work on the class side

Open jecisc opened this issue 5 years ago • 3 comments

I changed a method in a class and wanted to revert it. So I opened the History Browser and tried to install the latest committed version of the method. The problem is that I was in class side and the installed version was compiled on the instance side.

jecisc avatar Mar 28 '19 10:03 jecisc

This seems to be a bug between the IceLibgit**LogReader and the conversion to ring definitions.

(IceLibgitFiletreeLogReader fileName: aPath on: aCommit)
		package: aMethod package mcPackage; 
		packageDirectory: (self repository directoryPathStringForPackage: aMethod package);
		definitions)

The ring definition does not realize the method was actually from class side, because it checks only the full name of the class, and seems to expect a name like ABlah class.

The other related method is:


collectVersionsFrom: aCollection method: aMethod path: aPath
	| lastDefinition basepath history |
		
	basepath := self basepathOf: aPath.
	lastDefinition := MCMethodDefinition new.
	history := OrderedCollection new.
	aCollection reverseDo: [ :eachCommit | | definition |
		definition := self definitionFor: aMethod path: basepath commit: eachCommit.
		lastDefinition = definition  ifFalse: [ 
			1halt.
			history add: (IceLogVersion 
				commit: (self repository commitFromGitCommit: eachCommit)
				definition: definition asRingDefinition).
			lastDefinition := definition ] ].

	^ history reversed

guillep avatar Jun 09 '20 16:06 guillep

This changed method solves it.

IceTipVersionModel>>#basicInstall
    | definition cls |
    definition := self entity definition.
    cls := definition realClass.
    definition isMetaSide ifTrue: [ cls := cls class ].
    cls compile: definition sourceCode classified: definition category

I find it a bit strange that RGMethodDefinition shows the method in the form: Hobby>>#current instead of Hobby class>>#current, despite isMetaSide being true. I would also expect that definition realClass already would render the metaClass. Probably leading us into rabbit holes ;-)

ironirc avatar Jun 09 '23 16:06 ironirc

Tx :)

Ducasse avatar Jun 11 '23 09:06 Ducasse