Walkback - #nextChildNodeStartingFrom:suchThat:ifNone: committing selected code to branch (repo with ./dev/src)

[macta]

In a new image with the latest Iceberg 1.2.1 - from master I created some testcases for exercism but also modified some comments in some other classes.

As I wanted a clean PR for the exercises, I created a new local branch (leap-exercise) and then I picked commit and selected only the class and the test case (I unticked the other changes). I got the following walkback.

UndefinedObject(Object)>>doesNotUnderstand: #nextChildNodeStartingFrom:suchThat:ifNone:
IceNode>>firstNodeSuchThat:ifNone:
IceNode>>firstNodeSuchThat:
[ tree
firstNodeSuchThat: [ :definition | self isCodeSubdirectory: definition ] ] in IceDiff>>codeSubdirectoryNode in Block: [ tree...
BlockClosure>>cull:
ByteString(Collection)>>ifNotEmpty:ifEmpty:
IceDiff>>codeSubdirectoryNode
IceDiff>>includesPackageNamed:
[ :each | (aDiff includesPackageNamed: each name) not ] in IceWorkingCopy>>markAsCleanPackagesNotInDiff: in Block: [ :each | (aDiff includesPackageNamed: each name) ...etc...
[ :each |
(aBlock value: each)
ifTrue: [ each beClean ] ] in IceWorkingCopy>>markAsCleanPackagesThat: in Block: [ :each | ...
Array(SequenceableCollection)>>do:
IceWorkingCopy>>markAsCleanPackagesThat:
IceWorkingCopy>>markAsCleanPackagesNotInDiff:
IceWorkingCopy>>refreshDirtyPackages
IceLibgitRepository(IceRepository)>>commitIndexWithMessage:andParents:
IceWorkingCopy>>commitChanges:withMessage:
IceLibgitRepository(IceRepository)>>commitChanges:withMessage:
[ self repository
commitChanges: (diff copyWithOnly: items)
withMessage: message ] in IceTipCommitAction>>basicExecute in Block: [ self repository...
[ :bar |
bar label: aString.
aBlock value ] in MorphicUIManager(UIManager)>>informUser:during: in Block: [ :bar | ...
[ :bar | aBlock value: bar ] in MorphicUIManager>>informUserDuring: in Block: [ :bar | aBlock value: bar ]
BlockClosure>>cull:
[ ^ block cull: self ] in [ self prepareForRunning.
CurrentJob value: self during: [ ^ block cull: self ] ] in Job>>run in Block: [ ^ block cull: self ]
[ activeProcess psValueAt: index put: anObject.
aBlock value ] in CurrentJob(DynamicVariable)>>value:during: in Block: [ activeProcess psValueAt: index put: anObject....
BlockClosure>>ensure:
CurrentJob(DynamicVariable)>>value:during:
CurrentJob class(DynamicVariable class)>>value:during:
[ self prepareForRunning.
CurrentJob value: self during: [ ^ block cull: self ] ] in Job>>run in Block: [ self prepareForRunning....
BlockClosure>>ensure:
Job>>run
MorphicUIManager(UIManager)>>displayProgress:from:to:during:

[macta]

The IceNode which is asking for a parent shows as an IceNoModification in the debugger however it has two children which are the packages that have modifications.

Should this be self childrenDo: vs self parent?

[macta]

I can confirm its not as simple as that - as #nextChildNodeStartingFrom: anIceNode suchThat: conditionBlock ifNone: noneBlock diverts back up to the parent again and you have the problem all over again.

There is something not right in all of this.

[macta]

I think I can see the problem - you never tested this with a project that is more than 1 directory deep - in my case './dev/src' and not the normal './src'

In the method :

IceDiff>isCodeSubdirectory:

nodeLocation = "Path / 'Users' / 'macta' / 'Dev' / 'Smalltalk' / 'Pharo' / 'Pharo 6.1 - 64bit (tech preview)-ExercismDev-2' / 'pharo-local' / 'iceberg' / 'exercism' / 'pharo' / 'dev/src'"

(note that last path element is a string with 2 directories in it!!!)

repositorySubdirectoryLocation = '"Path / 'Users' / 'macta' / 'Dev' / 'Smalltalk' / 'Pharo' / 'Pharo 6.1 - 64bit (tech preview)-ExercismDev-2' / 'pharo-local' / 'iceberg' / 'exercism' / 'pharo' / 'dev' / 'src'"

Note in the latter its parsed into seperate path elements.

Hence they aren't equal!

Changing the equality check to gives the correct result (but i wonder if paths should do equality better?)

^ nodeLocation pathString = repositorySubdirectoryLocation pathString

[macta]

There is also another issue - IceNode>firstNodeSuchThat: conditionBlock ifNone: noneBlock

should better handle the null parent case

e.g. at the end do the nil check?

^ self parent ifNil: noneBlock ifNotNil: [ :p | p
	nextChildNodeStartingFrom: self
	suchThat: conditionBlock
	ifNone: noneBlock ]

[guillep]

I can easily add tests for the complex subdirectory tomorrow morning. I know there are some differences between paths in pharo 6 and pharo 7, I don't remember now, but I'll check tomorrow.

Thanks for the energy debugging it! ^^

[guillep]

I've already extended the test cases to add this case. As I thought, this is failing in Pharo6 but not Pharo7.

See build: https://travis-ci.org/guillep/iceberg/builds/414436413

[macta]

What about the other element of this - if you ever do drop out with no matching src directories you get nil parent error?

[guillep]

guillep@3432004 ? :)

[macta]

Ah yes - I didn't review the pr properly - sorry about that.