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 <marcus.den...@inria.fr> wrote: > > > >> On 23 Aug 2018, at 15:56, Tim Mackinnon <tim@testit.works> 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 <marcus.den...@inria.fr> wrote: >>> >>> >>> >>>> On 22 Aug 2018, at 16:24, Tim Mackinnon <tim@testit.works> 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 <marcus.den...@inria.fr> 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 <tim@testit.works> 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 <marcus.den...@inria.fr> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 17 Aug 2018, at 14:20, Tim Mackinnon <tim@testit.works> 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 <marcus.den...@inria.fr> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> On 17 Aug 2018, at 13:00, Tim Mackinnon <tim@testit.works> 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 <marcus.den...@inria.fr> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> On 10 Aug 2018, at 23:16, Tim Mackinnon <tim@testit.works> 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 >>>>> >>>>> >>>> >>>> >>> >>> >> > >
