Add experimental support for instance mode #5
Add experimental support for instance mode #5pietroppeter merged 11 commits intopietroppeter:masterfrom
Conversation
|
@pietroppeter I noticed you're not watching your own repository, so I guess it's possible you'll overlook this. |
Ah wow, indeed I got the notification only when you mentioned me. Now I should be watching it. Thanks so much for this! Need to look at it and give you feedback ( |
|
The majority of the logic can be moved to a submodule though! I just put it there for convenience at the moment. And given that we use a |
…tures The added doc comments should explain well enough how it works: - `globalAndLocal` wraps all procedures to also provide method taking `P5Instance`. While doing so, all names are added to a CacheSeq - an `instance` template that shadows existing helper templates such that closures are generated and added to the field - in each template call, the bodies are checked for possible replacements of calls / symbols that need a `p5Inst` first argument
The types must be in a separate files now so that we can import them in the instance logic file.
Also updates the instances example accordingly.
074526d to
3f67e20
Compare
|
Just rebased on current master, moved the instance macro logic to a submodule and extended support for In principle I consider this ready for merge, with the remaining optional TODOs left:
|
|
I will check this out and likely merge it. I want to test it out to have two instances in the same document and possibly create a nimib sugar for those (which includes automatically generated ids for divs). It seems reasonable to have only a partial subset of the functionalities now, we will add them as we go (most of the functionalities in the bindings are still untested for me, since they have no example where they are used). |
|
ok, I checked this and tested the two instances. In order for that to work the divName must be a as another minor changes: used @Vindaar, hope you do not mind I committed directly here, seemed easier. If ok for you I would merge this, close the instance issue and open a new one with the improvements you listed above. |
|
ah, another thing I noticed is there was a |
|
Ah, no. that import is just an accidental left over, because I started writing the prototype code in the example. Thanks for noticing. I'll take it out later. edit: you can of course also remove it! I don't mind you committing here at all, no worries! |
|
Already removed. Ready to merge on my side. |
|
merging, thanks again a lot for this great work @Vindaar ! |
After my brain was refreshed from exercising I decided to take a stab at implementing this.
I'm pretty happy with the results! We add a type for a
P5Instancewhich contains all symbols that are accessible from theinstancemacro. For each procedure that is to be callable, we generate them from the existing wrapper by using a newglobalAndLocalmacro that takes definitions and outputs two versions of them:P5Instanceas first argument, that is to be emitted to JS asp5Inst.<name>(<args>)(this is important, as the procedures are methods of the instance, as far as I understand so far)The new
instancetemplate shadows the existing sugar templates with versions that generate a new closure and add it to the corresponding field of theP5Instanceobject (remaining templates need to be added as wrapper templates as well as as fields to theP5Instancetype definition).Within these templates the body is checked for possible replacements from two sources:
globalAndLocal(fornnkCallnodes {nnkDotExprwill be added later } )P5Instancetype (fornnkSym/nnkIdentnodes)This approach (that internally uses
CacheSeqsto store the symbols) has the advantage that we don't convert too little / too many things.See the example for what the resulting code looks like.