Hi guys - I’ve been hammering on the exercism project to get pharo shining over there… its been a good side distraction (there is lots of energy in that community to) and its made me really push my use of Iceberg & git as well as learn some fo the newish file reference stuff (still getting good at that - but like the approach),
Anyway - I’ve got an Alpha working, and users can pull down a zero conf pharo image and eval a little script that will get them up and running (which is pretty slick). So the story then goes - Exercism is trying to simulate TDD and help you learn a new language (aka Pharo or Go or Python etc). To this end, they’ve got an active community building up little test exercises with suites of tests that new users can run to help them learn the syntax/essence of the language track they’ve signed up to. You run the tests, develop your solution and then submit it to a community to get feedback and then progress. Pretty standard - we can play in this pond too - and it turns out that Tonel actually makes it pretty easy to submit readable solutions that will fit on their website. YAY. The trouble is - when you pull down a new exercise - I use the TonelReader to pull the code into your image - and I thought it would be cute to just pull in the TestCase so that users can simulate the full TDD cycle - where you can create things in the debugger… this is where it all began right? So you hit a clanger when you do this - and in a way its a bit of a legacy thing we’ve carried around (and possibly should fix better). When Tonel reads in the TesCase, it normally will reference a class this isn’t there (I’ve deliberately left the solution out). It does the right thing and put a nil placeholder in the code so that it can read it in. HOWEVER - when you run your test and hit that nil class placeholder we do a very bad job of dealing with this in the debugger. And if you think about it - we also do a bad job when typing in code too - we essentially insist that declare a class then and there - unlike a selector which can happily be late bound. So back to my TDD example - user gets a debugger with a nil class error - the create button is actually not helpful for you as it only creates methods. So our “live in the debugger” mantra is a bit less obvious here. What you had to do is make a change to the source (like a space) so that you can reserve the method - which then causes you to get the “create class prompt”. So we have the ability - just don’t expose it very well - however now you have a missing method - but again its not so obvious that you have to resume your debugger (it hasn’t resumed when you created the class) - and then you will get another error for the missing method that the create button will now resolve. This feels very clunky to me - and makes me feel like I’m mis-selling smalltalk where we have a bullet point about “Amazing for debugging…”. This feels fixable though right? I’m wondering about thoughts though before jumping in… Tim p.s. - we’re building some exercises for exercism and getting that process streamlined so hopefully many more people can help - and maybe we can augment the great learning courses/videos/books that we already have. |
+1 On Tue, 7 Aug 2018 at 09:02 Tim Mackinnon <[hidden email]> wrote: Hi guys - I’ve been hammering on the exercism project to get pharo shining over there… its been a good side distraction (there is lots of energy in that community to) and its made me really push my use of Iceberg & git as well as learn some fo the newish file reference stuff (still getting good at that - but like the approach), Sent from the past
|
Feenk re-imagining aside (which I will pursue with them), it seems like our current tools can support this better right?
I see two issues: 1) if we encounter a not present class can we fix the debugger to offer something like we do for a missing method so it’s less obtuse? 2) when coding - if you want reference a missing class, why don’t we let you? TonelReader seems to do it, why can’t the editor? (This probably applies to variables as well - show them broken, let me fix it when I choose. The iVar case is a little rarer - although I hate the way we prompt fix, prompt fix instead of doing it in one go - it’s very old fashioned) Does anyone have tips on solving these? It spoils the exercism experience that I thought we could convey, so I’d like to at least fix #1 in 6.1 if I can. Tim
Sent from my iPhone
|
Right now we compile an “Undeclared” variable: a bining #name -> nil that lives in the Undeclareds dictionary. The result is that this variable is nil when read. This seems a good behaviour for non-interactive use, but not the right thing when doing development. In Pharo, these bindings are actually now not just Associations, but there is a class Hierarchy. The object describing the global binding is actually a UndeclaredVariable. You can see that like this: compile this with a non-existing “MyClass” (see below, Pharo7 can do it even in interactive mode): test ^MyClass. then (TT>>#test) literals first class Now: the compiler actually asks that object to generate the code for the read. If you look at class UndeclaredVariable, it looks like this: emitValue: methodBuilder methodBuilder pushLiteralVariable: self So we could do an experiment: couldn’t we generate code here that actually starts an interaction? Similar to the one (see below) that we do when the user tries to compile a method with an Undeclared in the first place (see below).
This is fixed in Pharo7: we added a menu entry “leave undeclared” as the first option:
I will back port the fix for 2) to Pharo6 and will do a quick prototype for 1)
|
In reply to this post by Tim Mackinnon
Hi Tim, I can only tell you that I agree with you ^^. Last year we worked with Stef and Luc on the idea of undefined classes (https://hal.archives-ouvertes.fr/hal-01585305/document). The code is available in smalltalkhub if I remember well (check the links in the paper). We did not integrate it because all the changes in Pharo were already a lot (new contribution process, moving to Git, new CI, and lots of other changes). It also means that it would require some more iterations, because we used it to download big and old projects, but never really tested it for TDD :) But if you would like, it's there to give it a try Guille On Thu, Aug 9, 2018 at 8:21 AM Tim Mackinnon <[hidden email]> wrote:
|
In reply to this post by Marcus Denker-4
>
> >> 2) when coding - if you want reference a missing class, why don’t we let you? TonelReader seems to do it, why can’t the editor? (This probably applies to variables as well - show them broken, let me fix it when I choose. The iVar case is a little rarer - although I hate the way we prompt fix, prompt fix instead of doing it in one go - it’s very old fashioned) >> > An alternative could be to not show the menu at all. Instead just compile (I like the ideas that “everything can be compiled and run”). Just compile the Undeclared and instead of breaking the flow, we should add a menu to the editor to do the “define Class” “Define Trait”… (with the gutter icons, we can provide a menu easily). I will make a small demo for that, too. Marcus |
> On 9 Aug 2018, at 10:06, Marcus Denker <[hidden email]> wrote: > >> >> >>> 2) when coding - if you want reference a missing class, why don’t we let you? TonelReader seems to do it, why can’t the editor? (This probably applies to variables as well - show them broken, let me fix it when I choose. The iVar case is a little rarer - although I hate the way we prompt fix, prompt fix instead of doing it in one go - it’s very old fashioned) >>> >> > > > An alternative could be to not show the menu at all. Instead just compile (I like the ideas that “everything can be compiled and run”). > > Just compile the Undeclared and instead of breaking the flow, we should add a menu to the editor to do the “define Class” “Define Trait”… > (with the gutter icons, we can provide a menu easily). > > I will make a small demo for that, too. > ah, and this fits well with the idea to show at runtime a dialog for fixing a Undeclared: The “fix tool” would be the editor itself. It shows the menu in the gutter for the user to define a class, or the possibility to just edit and compile the method. Marcus |
In reply to this post by Marcus Denker-4
Marcus |
Hi Marcus, Is it wise to backport things that may break things like this? I'd prefer to put the energy on backporting the workaround to avoid the infinite debuggers... On Thu, Aug 9, 2018 at 10:50 AM Marcus Denker <[hidden email]> wrote:
|
maybe not, but it just took 5 minutes as it is very simple
Yes.
|
Guys - you’re awesome! It really shows why our environment is so powerful and why more people need to think this way!
For back porting - you don’t need to, as for exercism we are loading a script anyway, so I can load in some code until we’re ready for 7.0 I do agree that infinite debuggers is a pressing problem too (and sorry to distract on that one). Still - we want to present our best face when we go live on exercism - and the pharo features list I copied from Pharo.org (slightly tweaked) drove me to live up to the Hugh standards we set ourselves. I’ll definitely try out what you’ve all proposed - I figured if was all possible . Tim
Sent from my iPhone
|
In reply to this post by Tim Mackinnon
On 7 August 2018 at 20:01, Tim Mackinnon <[hidden email]> wrote: Hi guys - I’ve been hammering on the exercism project to get pharo shining over there… its been a good side distraction (there is lots of energy in that community to) and its made me really push my use of Iceberg & git as well as learn some fo the newish file reference stuff (still getting good at that - but like the approach), For the purpose of Exercism, I think its reasonable for any class referenced by the TestCase to be loaded as an empty stub. Its not really giving the too much of a head start. If they miss seeing the magic of interactively auto-creating a class, oh well, there is plenty of other magic to show them. And if a solution needs a second class, then they might be exposed to it then. KISS. cheers -ben |
In reply to this post by Marcus Denker-4
Marcus, I love this idea, not showing the menu at all would be more aligned to live coding. As you said in a later e-mail it must be fixed at runtime, while running the tests. Thanks! On Thu, Aug 9, 2018 at 5:07 AM Marcus Denker <[hidden email]> wrote: > |
In reply to this post by Marcus Denker-4
Hey Marcus (or those knowledgable about stacks and contexts) - to self #1 - where the create button generates a class instead of a method, I’m a bit out of my depth.
I can see there is a DoesNotUnderstandDebugAction (which the create button calls). So to determine if the failure was due to a missing class or method - would this work? msg := self interruptedContext tempAt: 1. (msg lookupClass == UndefinedObject ) ifTrue: [ … create a class E.g. if you’re trying to send a msg to nil (which is what got patched into the compiled method for a missing class) then you are dealing with a missing class? AND then for create a class - how do I extract the name? I can get the compiled method for msg - but how do I marry that up with what you were executing? Am I write in thinking that the AST nodes and there locations might line up with the stack pointer somehow (or something like that)… if I can get that - then I get the name of the missing class and can then create it and then rerun the method right? This is very neat stuff I ‘ve never played with before. Tim
|
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…) Tim
|
I guess for a bit of historical record - I fixed the Pharo 6.1 image in exercism to properly handle the creation of a missing class. Not a lot of code (but you do need the right code - and it is slightly confusing how the debugger window is so tightly tied to the model which also has the stack and context - and thus getting rid of the window gets rid of the context - which in retrospect makes sense as closing the debugger should terminate everything).
I’ll kick it around a bit with Exercism - and if it seems good I’ll make a PR for Pharo 7 as Think its worth having? It does raise an interesting point however - when you hit a missing class, the debugger says “#message was sent to nil” - I would think we can do better than that given what Marcus discussed further below. If we were to save an UndefinedClass in the compiled method - then the error could be much better (not nil - but UndefinedClass, or UndefinedGlobal) - and equally the corrective action could be much better too. I think this is what Marcus was getting at - and I think it would be a much better thing for us to do. Having used one of his suggestions below (and tried it when coding some exercises) - I also think we should stop the disruptive prompting for missing classes or variables when you save and just show them as gutter warnings or squiggles in the editor. You could add some other key to walk you through fixes if you want to deal with them right at that moment. Anyway - this really shows how flexible our environment is (and the work we can keep doing). Tim
|
In reply to this post by Tim Mackinnon
> 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 - 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? Tim Sent from my iPhone
|
I will do that and have a look!
|
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)
Tim
|
Free forum by Nabble | Edit this page |