> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: > > The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) > Hm… AllExercismTests seems to not be there (just a green test in Welcome) Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) Marcus > Tim > >> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >> >> >> >>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>> >>> >>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>> >>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>> >> I will do that and have a look! >> >>> Tim >>> >>> >>> Sent from my iPhone >>> >>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>> >>>> >>>> >>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>> >>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>> >>>>> createMissingClassActionFor: aMessage in: aContext >>>>> |errorNode senderContext newClass variableNode | >>>>> senderContext := aContext sender. >>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>> variableNode := errorNode receiver receiver. >>>>> >>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>> aContext restart. >>>>> >>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>> >>>>> aContext restartWithNewReceiver: newClass >>>>> >>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>> >>>> >>>> >>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>> >>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>> >>>> Marcus >>>> >>>> >> > |
Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc).
In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) Tim > On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: > > > >> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >> >> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >> > > Hm… AllExercismTests seems to not be there (just a green test in Welcome) > > Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? > > It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) > > Marcus > > >> Tim >> >>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>> >>> >>> >>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>> >>>> >>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>> >>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>> >>> I will do that and have a look! >>> >>>> Tim >>>> >>>> >>>> Sent from my iPhone >>>> >>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>> >>>>> >>>>> >>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>> >>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>> >>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>> |errorNode senderContext newClass variableNode | >>>>>> senderContext := aContext sender. >>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>> variableNode := errorNode receiver receiver. >>>>>> >>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>> aContext restart. >>>>>> >>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>> >>>>>> aContext restartWithNewReceiver: newClass >>>>>> >>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>> >>>>> >>>>> >>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>> >>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>> >>>>> Marcus >>>>> >>>>> >>> >> > > |
Hi,
I played with it, nice! I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. Marcus > On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: > > Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). > > In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 > It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) > > Tim > > >> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >> >> >> >>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>> >>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>> >> >> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >> >> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >> >> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >> >> Marcus >> >> >>> Tim >>> >>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>> >>>> >>>> >>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>> >>>>> >>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>> >>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>> >>>> I will do that and have a look! >>>> >>>>> Tim >>>>> >>>>> >>>>> Sent from my iPhone >>>>> >>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>> >>>>>> >>>>>> >>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>> >>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>> >>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>> |errorNode senderContext newClass variableNode | >>>>>>> senderContext := aContext sender. >>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>> variableNode := errorNode receiver receiver. >>>>>>> >>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>> aContext restart. >>>>>>> >>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>> >>>>>>> aContext restartWithNewReceiver: newClass >>>>>>> >>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>> >>>>>> >>>>>> >>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>> >>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>> >>>>>> Marcus >>>>>> >>>>>> >>>> >>> >> >> > > |
Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong?
I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. Tim Sent from my iPhone Sent from my iPhone > On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: > > Hi, > > I played with it, nice! > > I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. > > Marcus > > >> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >> >> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >> >> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >> >> Tim >> >> >>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>> >>> >>> >>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>> >>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>> >>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>> >>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>> >>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>> >>> Marcus >>> >>> >>>> Tim >>>> >>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>> >>>>> >>>>> >>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>> >>>>>> >>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>> >>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>> I will do that and have a look! >>>>> >>>>>> Tim >>>>>> >>>>>> >>>>>> Sent from my iPhone >>>>>> >>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>> >>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>> >>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>> senderContext := aContext sender. >>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>> >>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>> aContext restart. >>>>>>>> >>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>> >>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>> >>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>> >>>>>>> >>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>> >>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>> >>>>>>> Marcus > > |
> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: > > Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks are always identity checks… Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? Marcus > > I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. > > Tim > > Sent from my iPhone > > > > Sent from my iPhone >> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >> >> Hi, >> >> I played with it, nice! >> >> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >> >> Marcus >> >> >>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>> >>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>> >>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>> >>> Tim >>> >>> >>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>> >>>> >>>> >>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>> >>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>> >>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>> >>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>> >>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>> >>>> Marcus >>>> >>>> >>>>> Tim >>>>> >>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>> >>>>>>> >>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>> >>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>> I will do that and have a look! >>>>>> >>>>>>> Tim >>>>>>> >>>>>>> >>>>>>> Sent from my iPhone >>>>>>> >>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>> >>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>> senderContext := aContext sender. >>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>> >>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>> aContext restart. >>>>>>>>> >>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>> >>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>> >>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>> >>>>>>>> >>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>> >>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>> >>>>>>>> Marcus >> >> > > |
Hi Marcus - that’s actually what I do - and “create” in this case, creates a class and then restarts like the method case does.
I guess I was wondering if we can do it more cleanly and also improve the debugger message. If I’ve understood you guys correctly- you try to remove the ambiguity around operations. Looking up a class and getting nil - seems like one of these holes you keep sorting out. I think the flaw in my solution is understanding if that message was being sent to a class, or some other global? I dont think I got that bit right (but it’s certainly better than nothing). e.g. in the debugger I am doing (in DoesNotUnderstandDebugAction) msg := self interruptedContext tempAt: 1. (msg lookupClass == UndefinedObject ) ifTrue: [ ^self createMissingClassIn: self interruptedContext ]. I’m not totally convinced that lookupClass has to be a class - although maybe its good enough. But really, at the time this happened - we probably knew better than to get a DNU debug action in the the first place - and equally the title in the debugger could be something more akin to the kind of action its supposed to be. Anyway - this is all musing on my part - and I will assemble a proper PR for review by you guys (and at least it advances us forward - and maybe opens the door to better changes further on). I’m just juggling another change at the moment - so it will be a few days. Tim Sent from my iPhone > On 23 Aug 2018, at 05:33, Marcus Denker <[hidden email]> wrote: > > > >> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: >> >> Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? > > In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks > are always identity checks… > > Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? > > Marcus > >> >> I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. >> >> Tim >> >> Sent from my iPhone >> >> >> >> Sent from my iPhone >>> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >>> >>> Hi, >>> >>> I played with it, nice! >>> >>> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >>> >>> Marcus >>> >>> >>>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>>> >>>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>>> >>>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>>> >>>> Tim >>>> >>>> >>>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>>> >>>>> >>>>> >>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>>> >>>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>>> >>>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>>> >>>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>>> >>>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>>> >>>>> Marcus >>>>> >>>>> >>>>>> Tim >>>>>> >>>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>>> >>>>>>>> >>>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>>> >>>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>>> I will do that and have a look! >>>>>>> >>>>>>>> Tim >>>>>>>> >>>>>>>> >>>>>>>> Sent from my iPhone >>>>>>>> >>>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>> >>>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>>> >>>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>>> senderContext := aContext sender. >>>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>>> >>>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>>> aContext restart. >>>>>>>>>> >>>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>>> >>>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>>> >>>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>>> >>>>>>>>> >>>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>>> >>>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>>> >>>>>>>>> Marcus >>> >>> >> >> > > |
> On 23 Aug 2018, at 15:56, Tim Mackinnon <[hidden email]> wrote: > > Hi Marcus - that’s actually what I do - and “create” in this case, creates a class and then restarts like the method case does. > Yes, that I saw. But I mean a different case: Imagine you do have code like nil doSomething or an expression that evaluates to nil, press “define” and get a method definition dialog, not the error message? (it is, as I mentioned, not that important as nobody ever wants to define a method in UndefinedObject, but for consistency it would be nice) > I guess I was wondering if we can do it more cleanly and also improve the debugger message. > > If I’ve understood you guys correctly- you try to remove the ambiguity around operations. Looking up a class and getting nil - seems like one of these holes you keep sorting out. > > I think the flaw in my solution is understanding if that message was being sent to a class, or some other global? I dont think I got that bit right (but it’s certainly better than nothing). > > e.g. in the debugger I am doing (in DoesNotUnderstandDebugAction) > > msg := self interruptedContext tempAt: 1. > (msg lookupClass == UndefinedObject ) ifTrue: [ > ^self createMissingClassIn: self interruptedContext ]. > > > I’m not totally convinced that lookupClass has to be a class - although maybe its good enough. But really, at the time this happened - we probably knew better than to get a DNU debug action in the the first place - and equally the title in the debugger could be something more akin to the kind of action its supposed to be. > > Anyway - this is all musing on my part - and I will assemble a proper PR for review by you guys (and at least it advances us forward - and maybe opens the door to better changes further on). > > I’m just juggling another change at the moment - so it will be a few days. > > Tim > > Sent from my iPhone > >> On 23 Aug 2018, at 05:33, Marcus Denker <[hidden email]> wrote: >> >> >> >>> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: >>> >>> Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? >> >> In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks >> are always identity checks… >> >> Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? >> >> Marcus >> >>> >>> I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. >>> >>> Tim >>> >>> Sent from my iPhone >>> >>> >>> >>> Sent from my iPhone >>>> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >>>> >>>> Hi, >>>> >>>> I played with it, nice! >>>> >>>> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >>>> >>>> Marcus >>>> >>>> >>>>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>>>> >>>>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>>>> >>>>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>>>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>>>> >>>>> Tim >>>>> >>>>> >>>>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>>>> >>>>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>>>> >>>>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>>>> >>>>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>>>> >>>>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>>>> >>>>>> Marcus >>>>>> >>>>>> >>>>>>> Tim >>>>>>> >>>>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>>>> >>>>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>>>> I will do that and have a look! >>>>>>>> >>>>>>>>> Tim >>>>>>>>> >>>>>>>>> >>>>>>>>> Sent from my iPhone >>>>>>>>> >>>>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>> >>>>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>>>> >>>>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>>>> senderContext := aContext sender. >>>>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>>>> >>>>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>>>> aContext restart. >>>>>>>>>>> >>>>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>>>> >>>>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>>>> >>>>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>>>> >>>>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>>>> >>>>>>>>>> Marcus >>>> >>>> >>> >>> >> >> > |
I’ve submitted a PR for what I did, so at least it improves the situation (but potentially could get even more refined as you suggest)
Tim > On 23 Aug 2018, at 11:26, Marcus Denker <[hidden email]> wrote: > > > >> On 23 Aug 2018, at 15:56, Tim Mackinnon <[hidden email]> wrote: >> >> Hi Marcus - that’s actually what I do - and “create” in this case, creates a class and then restarts like the method case does. >> > > Yes, that I saw. > > But I mean a different case: Imagine you do have code like > > nil doSomething > > or an expression that evaluates to nil, press “define” and get a method definition dialog, not the error message? > (it is, as I mentioned, not that important as nobody ever wants to define a method in UndefinedObject, but for consistency it would be nice) > > >> I guess I was wondering if we can do it more cleanly and also improve the debugger message. >> >> If I’ve understood you guys correctly- you try to remove the ambiguity around operations. Looking up a class and getting nil - seems like one of these holes you keep sorting out. >> >> I think the flaw in my solution is understanding if that message was being sent to a class, or some other global? I dont think I got that bit right (but it’s certainly better than nothing). >> >> e.g. in the debugger I am doing (in DoesNotUnderstandDebugAction) >> >> msg := self interruptedContext tempAt: 1. >> (msg lookupClass == UndefinedObject ) ifTrue: [ >> ^self createMissingClassIn: self interruptedContext ]. >> >> >> I’m not totally convinced that lookupClass has to be a class - although maybe its good enough. But really, at the time this happened - we probably knew better than to get a DNU debug action in the the first place - and equally the title in the debugger could be something more akin to the kind of action its supposed to be. >> >> Anyway - this is all musing on my part - and I will assemble a proper PR for review by you guys (and at least it advances us forward - and maybe opens the door to better changes further on). >> >> I’m just juggling another change at the moment - so it will be a few days. >> >> Tim >> >> Sent from my iPhone >> >>> On 23 Aug 2018, at 05:33, Marcus Denker <[hidden email]> wrote: >>> >>> >>> >>>> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: >>>> >>>> Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? >>> >>> In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks >>> are always identity checks… >>> >>> Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? >>> >>> Marcus >>> >>>> >>>> I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. >>>> >>>> Tim >>>> >>>> Sent from my iPhone >>>> >>>> >>>> >>>> Sent from my iPhone >>>>> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >>>>> >>>>> Hi, >>>>> >>>>> I played with it, nice! >>>>> >>>>> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >>>>> >>>>> Marcus >>>>> >>>>> >>>>>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>>>>> >>>>>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>>>>> >>>>>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>>>>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>>>>> >>>>>> Tim >>>>>> >>>>>> >>>>>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>>>>> >>>>>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>>>>> >>>>>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>>>>> >>>>>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>>>>> >>>>>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>>>>> >>>>>>> Marcus >>>>>>> >>>>>>> >>>>>>>> Tim >>>>>>>> >>>>>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>>>>> >>>>>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>>>>> I will do that and have a look! >>>>>>>>> >>>>>>>>>> Tim >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Sent from my iPhone >>>>>>>>>> >>>>>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>>> >>>>>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>>>>> >>>>>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>>>>> senderContext := aContext sender. >>>>>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>>>>> >>>>>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>>>>> aContext restart. >>>>>>>>>>>> >>>>>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>>>>> >>>>>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>>>>> >>>>>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>>>>> >>>>>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>>>>> >>>>>>>>>>> Marcus >>>>> >>>>> >>>> >>>> >>> >>> >> > > |
Thanks! I will have a look at the sprint tomorrow.
> On 30 Aug 2018, at 07:11, Tim Mackinnon <[hidden email]> wrote: > > I’ve submitted a PR for what I did, so at least it improves the situation (but potentially could get even more refined as you suggest) > > Tim > >> On 23 Aug 2018, at 11:26, Marcus Denker <[hidden email]> wrote: >> >> >> >>> On 23 Aug 2018, at 15:56, Tim Mackinnon <[hidden email]> wrote: >>> >>> Hi Marcus - that’s actually what I do - and “create” in this case, creates a class and then restarts like the method case does. >>> >> >> Yes, that I saw. >> >> But I mean a different case: Imagine you do have code like >> >> nil doSomething >> >> or an expression that evaluates to nil, press “define” and get a method definition dialog, not the error message? >> (it is, as I mentioned, not that important as nobody ever wants to define a method in UndefinedObject, but for consistency it would be nice) >> >> >>> I guess I was wondering if we can do it more cleanly and also improve the debugger message. >>> >>> If I’ve understood you guys correctly- you try to remove the ambiguity around operations. Looking up a class and getting nil - seems like one of these holes you keep sorting out. >>> >>> I think the flaw in my solution is understanding if that message was being sent to a class, or some other global? I dont think I got that bit right (but it’s certainly better than nothing). >>> >>> e.g. in the debugger I am doing (in DoesNotUnderstandDebugAction) >>> >>> msg := self interruptedContext tempAt: 1. >>> (msg lookupClass == UndefinedObject ) ifTrue: [ >>> ^self createMissingClassIn: self interruptedContext ]. >>> >>> >>> I’m not totally convinced that lookupClass has to be a class - although maybe its good enough. But really, at the time this happened - we probably knew better than to get a DNU debug action in the the first place - and equally the title in the debugger could be something more akin to the kind of action its supposed to be. >>> >>> Anyway - this is all musing on my part - and I will assemble a proper PR for review by you guys (and at least it advances us forward - and maybe opens the door to better changes further on). >>> >>> I’m just juggling another change at the moment - so it will be a few days. >>> >>> Tim >>> >>> Sent from my iPhone >>> >>>> On 23 Aug 2018, at 05:33, Marcus Denker <[hidden email]> wrote: >>>> >>>> >>>> >>>>> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: >>>>> >>>>> Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? >>>> >>>> In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks >>>> are always identity checks… >>>> >>>> Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? >>>> >>>> Marcus >>>> >>>>> >>>>> I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. >>>>> >>>>> Tim >>>>> >>>>> Sent from my iPhone >>>>> >>>>> >>>>> >>>>> Sent from my iPhone >>>>>> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> I played with it, nice! >>>>>> >>>>>> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >>>>>> >>>>>> Marcus >>>>>> >>>>>> >>>>>>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>>>>>> >>>>>>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>>>>>> >>>>>>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>>>>>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>>>>>> >>>>>>> Tim >>>>>>> >>>>>>> >>>>>>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>>>>>> >>>>>>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>>>>>> >>>>>>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>>>>>> >>>>>>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>>>>>> >>>>>>>> Marcus >>>>>>>> >>>>>>>> >>>>>>>>> Tim >>>>>>>>> >>>>>>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>>>>>> >>>>>>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>>>>>> I will do that and have a look! >>>>>>>>>> >>>>>>>>>>> Tim >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Sent from my iPhone >>>>>>>>>>> >>>>>>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>>>>>> >>>>>>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>>>>>> senderContext := aContext sender. >>>>>>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>>>>>> >>>>>>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>>>>>> aContext restart. >>>>>>>>>>>>> >>>>>>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>>>>>> >>>>>>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>>>>>> >>>>>>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>>>>>> >>>>>>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>>>>>> >>>>>>>>>>>> Marcus >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>> >> >> > > |
Hi,
I had a look: this works nicely! I even found a way to automatically fall back to the method creation, so nil doSomething on create will create a method. I have merged your code and will do a PR with the improvement later. Marcus > On 30 Aug 2018, at 09:57, Marcus Denker <[hidden email]> wrote: > > Thanks! I will have a look at the sprint tomorrow. > > >> On 30 Aug 2018, at 07:11, Tim Mackinnon <[hidden email]> wrote: >> >> I’ve submitted a PR for what I did, so at least it improves the situation (but potentially could get even more refined as you suggest) >> >> Tim >> >>> On 23 Aug 2018, at 11:26, Marcus Denker <[hidden email]> wrote: >>> >>> >>> >>>> On 23 Aug 2018, at 15:56, Tim Mackinnon <[hidden email]> wrote: >>>> >>>> Hi Marcus - that’s actually what I do - and “create” in this case, creates a class and then restarts like the method case does. >>>> >>> >>> Yes, that I saw. >>> >>> But I mean a different case: Imagine you do have code like >>> >>> nil doSomething >>> >>> or an expression that evaluates to nil, press “define” and get a method definition dialog, not the error message? >>> (it is, as I mentioned, not that important as nobody ever wants to define a method in UndefinedObject, but for consistency it would be nice) >>> >>> >>>> I guess I was wondering if we can do it more cleanly and also improve the debugger message. >>>> >>>> If I’ve understood you guys correctly- you try to remove the ambiguity around operations. Looking up a class and getting nil - seems like one of these holes you keep sorting out. >>>> >>>> I think the flaw in my solution is understanding if that message was being sent to a class, or some other global? I dont think I got that bit right (but it’s certainly better than nothing). >>>> >>>> e.g. in the debugger I am doing (in DoesNotUnderstandDebugAction) >>>> >>>> msg := self interruptedContext tempAt: 1. >>>> (msg lookupClass == UndefinedObject ) ifTrue: [ >>>> ^self createMissingClassIn: self interruptedContext ]. >>>> >>>> >>>> I’m not totally convinced that lookupClass has to be a class - although maybe its good enough. But really, at the time this happened - we probably knew better than to get a DNU debug action in the the first place - and equally the title in the debugger could be something more akin to the kind of action its supposed to be. >>>> >>>> Anyway - this is all musing on my part - and I will assemble a proper PR for review by you guys (and at least it advances us forward - and maybe opens the door to better changes further on). >>>> >>>> I’m just juggling another change at the moment - so it will be a few days. >>>> >>>> Tim >>>> >>>> Sent from my iPhone >>>> >>>>> On 23 Aug 2018, at 05:33, Marcus Denker <[hidden email]> wrote: >>>>> >>>>> >>>>> >>>>>> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: >>>>>> >>>>>> Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? >>>>> >>>>> In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks >>>>> are always identity checks… >>>>> >>>>> Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? >>>>> >>>>> Marcus >>>>> >>>>>> >>>>>> I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. >>>>>> >>>>>> Tim >>>>>> >>>>>> Sent from my iPhone >>>>>> >>>>>> >>>>>> >>>>>> Sent from my iPhone >>>>>>> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> I played with it, nice! >>>>>>> >>>>>>> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >>>>>>> >>>>>>> Marcus >>>>>>> >>>>>>> >>>>>>>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>>>>>>> >>>>>>>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>>>>>>> >>>>>>>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>>>>>>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>>>>>>> >>>>>>>> Tim >>>>>>>> >>>>>>>> >>>>>>>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>> >>>>>>>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>>>>>>> >>>>>>>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>>>>>>> >>>>>>>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>>>>>>> >>>>>>>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>>>>>>> >>>>>>>>> Marcus >>>>>>>>> >>>>>>>>> >>>>>>>>>> Tim >>>>>>>>>> >>>>>>>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>>>>>>> >>>>>>>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>>>>>>> I will do that and have a look! >>>>>>>>>>> >>>>>>>>>>>> Tim >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Sent from my iPhone >>>>>>>>>>>> >>>>>>>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>>>>>>> >>>>>>>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>>>>>>> senderContext := aContext sender. >>>>>>>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>>>>>>> >>>>>>>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>>>>>>> aContext restart. >>>>>>>>>>>>>> >>>>>>>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>>>>>>> >>>>>>>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>>>>>>> >>>>>>>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>>>>>>> >>>>>>>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>>>>>>> >>>>>>>>>>>>> Marcus >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>> >>> >> >> > |
Here is the new PR:
https://github.com/pharo-project/pharo/pull/1731 > On 31 Aug 2018, at 11:17, Marcus Denker <[hidden email]> wrote: > > Hi, > > I had a look: this works nicely! > > I even found a way to automatically fall back to the method creation, so > > nil doSomething > > on create will create a method. > > I have merged your code and will do a PR with the improvement later. > > Marcus > >> On 30 Aug 2018, at 09:57, Marcus Denker <[hidden email]> wrote: >> >> Thanks! I will have a look at the sprint tomorrow. >> >> >>> On 30 Aug 2018, at 07:11, Tim Mackinnon <[hidden email]> wrote: >>> >>> I’ve submitted a PR for what I did, so at least it improves the situation (but potentially could get even more refined as you suggest) >>> >>> Tim >>> >>>> On 23 Aug 2018, at 11:26, Marcus Denker <[hidden email]> wrote: >>>> >>>> >>>> >>>>> On 23 Aug 2018, at 15:56, Tim Mackinnon <[hidden email]> wrote: >>>>> >>>>> Hi Marcus - that’s actually what I do - and “create” in this case, creates a class and then restarts like the method case does. >>>>> >>>> >>>> Yes, that I saw. >>>> >>>> But I mean a different case: Imagine you do have code like >>>> >>>> nil doSomething >>>> >>>> or an expression that evaluates to nil, press “define” and get a method definition dialog, not the error message? >>>> (it is, as I mentioned, not that important as nobody ever wants to define a method in UndefinedObject, but for consistency it would be nice) >>>> >>>> >>>>> I guess I was wondering if we can do it more cleanly and also improve the debugger message. >>>>> >>>>> If I’ve understood you guys correctly- you try to remove the ambiguity around operations. Looking up a class and getting nil - seems like one of these holes you keep sorting out. >>>>> >>>>> I think the flaw in my solution is understanding if that message was being sent to a class, or some other global? I dont think I got that bit right (but it’s certainly better than nothing). >>>>> >>>>> e.g. in the debugger I am doing (in DoesNotUnderstandDebugAction) >>>>> >>>>> msg := self interruptedContext tempAt: 1. >>>>> (msg lookupClass == UndefinedObject ) ifTrue: [ >>>>> ^self createMissingClassIn: self interruptedContext ]. >>>>> >>>>> >>>>> I’m not totally convinced that lookupClass has to be a class - although maybe its good enough. But really, at the time this happened - we probably knew better than to get a DNU debug action in the the first place - and equally the title in the debugger could be something more akin to the kind of action its supposed to be. >>>>> >>>>> Anyway - this is all musing on my part - and I will assemble a proper PR for review by you guys (and at least it advances us forward - and maybe opens the door to better changes further on). >>>>> >>>>> I’m just juggling another change at the moment - so it will be a few days. >>>>> >>>>> Tim >>>>> >>>>> Sent from my iPhone >>>>> >>>>>> On 23 Aug 2018, at 05:33, Marcus Denker <[hidden email]> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On 22 Aug 2018, at 16:24, Tim Mackinnon <[hidden email]> wrote: >>>>>>> >>>>>>> Hi - but I guess my question is (and excuse my basic knowledge in this area) - when a class isn’t found - can we do better than return nil so that the debugger can give a better msg and presumably the code I’ve written could live on that undefined object? Or am thinking about this wrong? >>>>>> >>>>>> In pharo7 we could easily do that (due to the “binding”, the meta class of the variable) being different. We could return a nil subclass or add code into the method directly. But the problem with that is that nil checks >>>>>> are always identity checks… >>>>>> >>>>>> Could you not in the case you now raise the error just fall back to the “define method”, the behaviour we have now? >>>>>> >>>>>> Marcus >>>>>> >>>>>>> >>>>>>> I will also put together a pr for this in Pharo 7 if you think it’s a decent fix. >>>>>>> >>>>>>> Tim >>>>>>> >>>>>>> Sent from my iPhone >>>>>>> >>>>>>> >>>>>>> >>>>>>> Sent from my iPhone >>>>>>>> On 22 Aug 2018, at 09:51, Marcus Denker <[hidden email]> wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I played with it, nice! >>>>>>>> >>>>>>>> I guess the case when you really get a DNU on nil (and want to create method there) does not really happen… extending nil is for special cases. >>>>>>>> >>>>>>>> Marcus >>>>>>>> >>>>>>>> >>>>>>>>> On 22 Aug 2018, at 13:39, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>> >>>>>>>>> Sorry Marcus - you needed to follow the exercism instructions and right click on the exercism package to get an exercism menu to fetch a new exercise (e.g. hello-world). The is then using the TonalReader to pull in code - and then you get a test class that can reference a class that isn’t there yet. (But you need to have the exercism cli installed as per the instructions etc). >>>>>>>>> >>>>>>>>> In retrospect I think it might be simpler to download this 6.1 image that already has done that - https://www.dropbox.com/s/x2ot9f8arbbvlyb/PharoExercism.zip?dl=0 >>>>>>>>> It has TwoFerTest that is in that state. If you click on the TestWithName orb, you will see "#new was sent to nil” - can you can see how my Create button has been fixed per you suggestions to create a class. (The code I wrote is in ExercismTools:DoesNotUnderstandDebugAction>>createMissingClassIn:) >>>>>>>>> >>>>>>>>> Tim >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 22 Aug 2018, at 04:44, Marcus Denker <[hidden email]> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>> >>>>>>>>>>> The direct link to instructions is here: https://exercism.io/tracks/pharo/installation (not sure if you have to be signed up to see it otherwise its in the repo here: https://github.com/exercism/pharo/blob/master/docs/INSTALLATION.md) >>>>>>>>>> >>>>>>>>>> Hm… AllExercismTests seems to not be there (just a green test in Welcome) >>>>>>>>>> >>>>>>>>>> Is this supposed to contain the code below (the createMissingClassActionFor:in:) ? >>>>>>>>>> >>>>>>>>>> It would be nice to have an image that shows exactly the problem (I do not have that much time sadly to work on it,but I do have some time to check if I have an image that is set up to the point where i can easily recreate the problem) >>>>>>>>>> >>>>>>>>>> Marcus >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Tim >>>>>>>>>>> >>>>>>>>>>>> On 17 Aug 2018, at 07:17, Marcus Denker <[hidden email]> wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Hi Marcus - I can put an image somewhere if that helps (do you just need the .image and .changes)? >>>>>>>>>>>>> >>>>>>>>>>>>> Or you can repro from a fresh 6.1 if you follow the exercism Pharo instructions (https://exercism.io/tracks/pharo) to load the first hello world-world example and run the tests. This has my code changes to make create work with a nil class - but maybe we can do better? >>>>>>>>>>>> I will do that and have a look! >>>>>>>>>>>> >>>>>>>>>>>>> Tim >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Sent from my iPhone >>>>>>>>>>>>> >>>>>>>>>>>>>> On 17 Aug 2018, at 06:21, Marcus Denker <[hidden email]> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <[hidden email]> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Actually I think I figured that bit out - a bit clumsily - (pointers appreciated) >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> createMissingClassActionFor: aMessage in: aContext >>>>>>>>>>>>>>> |errorNode senderContext newClass variableNode | >>>>>>>>>>>>>>> senderContext := aContext sender. >>>>>>>>>>>>>>> errorNode := senderContext method sourceNodeExecutedForPC: senderContext pc. >>>>>>>>>>>>>>> variableNode := errorNode receiver receiver. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> newClass := OCUndeclaredVariableWarning new node: variableNode; defineClass: variableNode name. >>>>>>>>>>>>>>> aContext restart. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> However that last line is wrong, as it doesn’t restart with my newly defined class - I also tried >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> aContext restartWithNewReceiver: newClass >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> But again, I get a debugger where my class is still bound to nil. So what’s the trick to re-evaluate with the new class I’ve created? Or maybe I’m totally on the wrong track (still its very interesting…) >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> what is a bit bad is that you catch the problem “too late” (that is, the DNU to nil, not the read of nil), so nil is already pushed on the stack at this point. >>>>>>>>>>>>>> >>>>>>>>>>>>>> I tried it in the inspector and at least the class binding was correct after defining the class… do you have an image with the whole code to try? >>>>>>>>>>>>>> >>>>>>>>>>>>>> Marcus >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >>> >> > |
Free forum by Nabble | Edit this page |