nimib
nimib copied to clipboard
pietroppeter
implement container blocks
This is my attempt at implementing container blocks (#117) thus far.
TODO:
- [ ] Refactor all relevant blocks that have bodies to support container blocks.
- [ ] Rewrite
nbCollectAllNbJsto work with arbitrary numbers of nested levels of blocks. (recursive) - [ ] Replace
nimibCodewithnbCodein docs. - [ ] Verify that it doesn't break any current code
- [ ] Everything else I haven't thought of...
Refactor all relevant blocks that have bodies to support container blocks.
Can't we put useCurrentBlockAsContainer directly in newNbBlock?
It screws up captureStdout. captureStdout must be placed outside nnContainer or else nb.blk.output will point to the wrong block:/
To expand on this, take this psuedocode:
nbContainer:
captureStdout(nb.blk.output):
body # new blocks are create here, thus modifying nb.blk
# here we set the captured string:
nb.blk.output = readFile(tempfile)
# only here is nb.blk reset to the parent
The problem stems from the fact that captureStdout is a template and only receives the ident nb.blk.output, not the value nb.blk.output had when we are calling captureStdout. By the time we assign to it, it has changed value and is now pointing to another block's output field. What's worse is that we can't save the current nb.blk.output in a variable at the top of captureStdout (before running body) and assign to it afterword, because strings are value-types in Nim.
So if we want to use useCurrentBlockAsContainer directly in newNbBlock we would have to change captureStdout to take in the block instead (NbBlock is a ref object) so we can save it at the top and modify it thanks to the reference semantics. I think this is the way forward, yes we do limit captureStdout to not work with arbitrary strings, but I don't know why you would want to use captureStdout from nimib if it wasn't to display it in a block :shrug:
The alternative is that we would have to rewrite all blocks using captureStdout to save the current nb.blk manually and pass it instead:
nbContainer:
var blk = nb.blk
captureStdout(blk.output):
body
But this is something which is error prone if you forget about it and hard to debug.
Thanks for the explanations. It seems captureStdout plays a very special role and with these changes an even more special role (since we are introducing disabling it). I am wondering if maybe a better option would be to manage it at nbDoc level. We could have a sequence of outputs there where we add every time we catch. We then should find a way to save indices of capture for nbCode and any blocks that captures. It looks like this approach would break a lot more stuff though.
I just don't see how your proposed solution solves the problem we are having here. Namely how we can get the current block correctly so we can assign to it. Because if we could assign the index to the block, why wouldn't we just assign the string to the block directly instead? It's getting the block that is the problem, not where we store the strings.
The simplest solution is to just change so captureStdout accepts the block instead of the output field. Minimal changes needed nb.blk.output → nb.blk.
Ah yeah maybe it does not help here and it was an orthogonal thought and we can just discard it.
No problem in changing captureStdout to take a NbBlock, we might want to keep also the string version there since it might in general be useful to people even if it is not used anymore by nimib (it is in fusion but fusion is not in a good shape).
Ah yes that might work, maybe we don't require the ident to be untyped but can make one overload with NbBlock and one with string. The string version could be used by the NbBlock one. This would cause minimal breakage as current code would continue to work as before. Great idea! 😄
Mmh, but what about now that nbCode is/will be a container block we have outputs that are actually blocks? Every time you capture output you add it as a block. In this way we would be able to alternate output with images and other stuff. Not sure why I keep coming to this (I do not really feel it is a very important feature), I am just trying to find a more natural api (and I give it 50-50 that I am wrong again... ;))
I think I see what you mean. If we do
nbCode:
echo 1
nbshow img
echo 2
it should show the "1", then the image, and last the "2"? It would be nice, but how would we tell captureStdout to split the capture everytime we add a new block (and how do we know we have added a new block)?
how would we tell captureStdout to split the capture everytime we add a new block (and how do we know we have added a new block)?
Every time we end a capture (for example disabling it) we add a new block like we add a block when we show an image. If this is doable...
So nbShow and every block you would want to show interlaced with captured text would have to use disableCaptureStdout? What about the disableCaptureStdouts in newNbBlock, that would split any text at least 2 time 🤔 I'm not saying it doesn't work, I'm just not sure how it would work.
Hold on, I contradicted myself there, nbShow wouldn't have to manually use disableCapture as newNbBlock does it for us already. Then we just have the problem that we will be creating quite a few empty blocks if there isn't actually any blocks being created.
Basically we are not using anymore the output field of nbCode block
So far I'm following along, it's the specifics of how to split the capture into blocks that I'm having problems with 😅
Let's take my example from above:
nbCode:
echo 1
nbshow img
echo 2
nbShow being a block has two disableCaptureStdout (because it has one stdout.write before blockImpl and one after). This the list of blocks of the nbCode would become:
[
NbBlock("1") # psuedo code for the block containing the captured string "1",
# The first `disableCaptureStdout`
NbBlock("img"),
# The second `disableCaptureStdout`
# nbShow didn't capture anything so this one is empty:
NbBlock(""),
NbBlock("2")
]
It is the stray NbBlock("") that I'm thinking of. How would it be created and rendered? We will create one of these NbBlock("") for every block that is created in a container.
If we are going this route, I think we should have a dedicated template for stopping the capturing and resuming it, and not rely on disableCaptureStdout to handle it for us.
I'm still not entierly convinced that this is something that we want to pursue, it is quite different from the way we are currently doing it.
Well the block that captures nothing we can probably skip it (the moment we need to create the block that captures we check if it has actually captured something).
I do agree that we might want to have a better mechanism for managing captures. I would probably go at the level of managing this at NbDoc level than as a template but no concrete plans.
Incidentally, when I am thinking of trying to write some code about this, I am always thinking I want also to try out the NbBlock using inheritance (although unsure if it creates more problems than it solves, so I will try to resist the urge...)
Well the block that captures nothing we can probably skip it (the moment we need to create the block that captures we check if it has actually captured something).
And how do we check what a block has captured, if anything at all? :sweat_smile: It would require us to introduce some way of keeping track of it. There's just too many unknowns at the moment for me to get on this plan, I need concrete examples before I can try to understand :thinking: I think the current way is nice because it is simple and clear which blocks will capture what. So an alternative approach would in my mind require that as well.
I do agree that we might want to have a better mechanism for managing captures. I would probably go at the level of managing this at NbDoc level than as a template but no concrete plans.
Yes we need something to handle it, but I'm not sold on doing it at the NbDoc level. What advantages would it bring? Do we ever need to access the same capture more than once? My current understanding of it, is that it just complicates rendering by scattering the data in multiple places. In my ideal world, a block should itself contain all the information needed to render itself.
Incidentally, when I am thinking of trying to write some code about this, I am always thinking I want also to try out the NbBlock using inheritance (although unsure if it creates more problems than it solves, so I will try to resist the urge...)
Haha I've also been a bit tempted recently :rofl: But I've come to grasps with the fact that these two problems are completely orthogonal to each other so I'm leaving the object until after this is done hehe :sweat_smile: I would love to see some pseudocode from you though on an example (the one I have used for example) and how everything roughly would work :) I won't be able to do anything useful until I actually understand how you are thinking :rofl:
so I tried to make explicit my ideas here in this mini nimib (a minib 😛): https://gist.github.com/pietroppeter/493ff1b546f783d12c935e7fc1e25532
I did try the object hierarchy in the end, I agree it is orthogonal but it allows me to write easily actual code (not pseudocode) that does stuff. The only problem is that capturing is buggy, have not had time to fix it.
Also, your disableCapture template is fine (it would need to be adapteb), but we might also decide to skip logging when we are capturing output.
Wow thanks! :D And "minib" is a word we should use more often :sunglasses: Cool that you can write a barebones nimib in like 100 lines of code :exploding_head: I think I understand what you mean by doing the capturing in the NbDoc now :+1: We don't use a template to keep track of the state, but instead use fields in the doc object and discard the idea of "resuming" a capture. Every start of a capture will check if there is a current capture and create a output-block for it. This seems very clean! I assume the
let wasCapturing = nb.isCapturingOutput
if wasCapturing:
nbEndCapture
could be hidden away in a template (maybe directly in nbCaptureOutput?).
A bit off-topic, but for the object hierarchy, is this kind of rendering you want to do or will we keep the partials and contexts? Or I guess each block could decide for itself how it wants to be rendered.
Yep, I think now I explained myself better, you seem to have gotten what I vaguely meant before :)
And "minib" is a word we should use more often 😎
Yeah, I ran into the word at the end and love it already 🥰. Actually I think I will use mb instead of nb as a prefix so that we can write nimib blogposts about any minib we come up with without name clashes (it is about time I write another nblog post).
I assume the
let wasCapturing …could be hidden away in a template (maybe directly in nbCaptureOutput?).
I guess it could, no preferences on that. it is actually to be able to resume capturing if you have a nbCode inside another nbCode
A bit off-topic, but for the object hierarchy, is this kind of rendering you want to do or will we keep the partials and contexts? Or I guess each block could decide for itself how it wants to be rendered.
I do not have concrete ideas on how to render but the OOP technique should be rather flexible. Ideally we could get rid of block partials and renderProcs. It remains to be seen if we want to go down the road though, just by playing with it for minib I found some rigidity (initially my base block was empty and I had a container block inheriting from that but I was not able to make that work).
Another possible direction for a different minib is one where all blocks are of a simple type:
type
NbBlock = ref object
blocks: seq[NbBlock]
data: JsonNode
Yeah, I ran into the word at the end and love it already smiling_face_with_three_hearts. Actually I think I will use mb instead of nb as a prefix so that we can write nimib blogposts about any minib we come up with without name clashes (it is about time I write another nblog post).
That's a good idea, that would give people a good view into the basics of how nimib itself work as well. So it could work as a primer for people interested in contributing :D
I guess it could, no preferences on that. it is actually to be able to resume capturing if you have a nbCode inside another nbCode
Reducing boilerplate for writing a block that captures is a good thing in my book, it is less error-prone as you don't have to remember to add all these steps (check if capturing, end capture, start capture, remember to end capture when done). Exactly how we the API will look like is another question though.
Ideally we could get rid of block partials and renderProcs.
Unless we replaced these mechanics with something else, we would lose the ability to customize an existing block (using partials) and to reuse procs used by other blocks. We have an issue open in nimiSlides to move from nbRawHtml to real blocks for this exact reason. The procs would probably be manageable to get working in another way, but I don't see another way of customizing how an existing block is rendered :/ It's not like you can override the render method of NbCodeBlock, that would just cause anambigous symbol errors if defined in a separate library.
Another possible direction for a different minib is one where all blocks are of a simple type:
Yes that's very reminiscent of the way we are doing it now but with different types, so this would be a good one to showcase if we keep the current way.
What is the take-away from these discussions? :rofl: Should we wait a bit until we proceed with container blocks and do them at the same time we switch to OO blocks? It is 2023H2 goal after all.
Well I always thought container blocks could come before the refactoring. Also still not sure about going the OO way, so I would keep the two things separate. No pressure on trying to get container blocks out now. We definitely found out they are more complex then expected (especially because of the capturing output part). We could actually implement container blocks but with limitations on output capture (you cannot/should not put nbCode inside nbCode), maybe that's a good compromise for now.
this anticipated discussion on container blocks though started because of something we wanted to do 2023H1 (NbShow #74) and there we argued container blocks could be an enabler for a better api for that.
Well I always thought container blocks could come before the refactoring. Also still not sure about going the OO way, so I would keep the two things separate. No pressure on trying to get container blocks out now. We definitely found out they are more complex then expected (especially because of the capturing output part). We could actually implement container blocks but with limitations on output capture (you cannot/should not put nbCode inside nbCode), maybe that's a good compromise for now
Ok, as adding container blocks without taking capturing into account doesn't change any defined old behavior, it should be ok to add it in a minor release. What's still to be decided though is if we actually want a nbUseCurrentBlockAsContainer template that you have to use in all blocks supporting containers. We still need it to handle capturing correctly regardless of if we do nested capturing or not...
this anticipated discussion on container blocks though started because of something we wanted to do 2023H1 (NbShow https://github.com/pietroppeter/nimib/issues/74) and there we argued container blocks could be an enabler for a better api for that.
Haha true, maybe I should start in that end :rofl: I'll not be available during the weekend, so I'll probably not be able to get any work done until next weekend. Maybe I will let this PR sit for a few weeks.
Everything else I haven't thought of...
Update allblocks.nim in docsrc to document the new block!
