pharo-spec / NewTools

All development tools for Pharo, developed with Spec
21 stars 53 forks source link

[Debugger][P12] Recompiling code within a bloc cannot be done more than once in the debugger #722

Closed adri09070 closed 3 months ago

adri09070 commented 6 months ago

If you have a bloc closure like this, called via a method

MyClass>>mySortBlock

    ^ [ :a :b | a < b ]

Then, you can paste this code in a playground and debug it (with one stepOver and one stepInto:

anObject := MyClass new.
anObject mySortBlock value: 1 value: 2

In the debugger, try to modify the code of the bloc, for example by adding a halt:

MyClass>>mySortBlock

      ^ [ :a :b | self halt.  a < b ]

and save the changes.

A popup will open, click on "Compile and browse".

A browser will then open on the embedding method with the new code:

image

Close this browser.

Then, in the debugger, change the code in the bloc again, for example by commenting the halt you've just added:

MyClass>>mySortBlock

      ^ [ :a :b | "self halt."  a < b ]

Save the changes, confirm the popup by clicking "compile and browse".

A browser opens on the method that hasn't been modified: the halt isn't commented:

image

Expected behavior:

The method should have been recompiled with the new code (i.e: with the commented halt)

Ducasse commented 3 months ago

Oh yes I got bitten by this bug.

adri09070 commented 3 months ago

Well, I've done the fix some months ago and it's still not integrated.

https://github.com/pharo-project/pharo/pull/16319 should be merged and then https://github.com/pharo-spec/NewTools/pull/723

@StevenCostiou

Ducasse commented 3 months ago

@adri09070 I do not understand because the issue is integrated.

Ducasse commented 3 months ago

I understand that you are not happy but if people do not review PRs then they do not get reviewed. And now this is difficult to follow what is happening because of the dependencies between projects. Now I do not get why the test is failing even if I merged https://github.com/pharo-project/pharo/pull/16319

adri09070 commented 3 months ago

Now, the test is green

adri09070 commented 3 months ago

Now, for the backport, do I need to create these PRs again targeting the Pharo12 branch or are you able to merge the same PR into the Pharo12 branch?