implement container blocks #177
Conversation
Can't we put useCurrentBlockAsContainer directly in newNbBlock? |
|
It screws up captureStdout. captureStdout must be placed outside nnContainer or else |
|
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 So if we want to use The alternative is that we would have to rewrite all blocks using nbContainer:
var blk = nb.blk
captureStdout(blk.output):
bodyBut 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 |
|
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 |
|
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 2it 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)? |
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 |
|
Basically we are not using anymore the output field of nbCode block |
|
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. |
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
[
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 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 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...) |
And how do we check what a block has captured, if anything at all? 😅 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 🤔 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.
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.
Haha I've also been a bit tempted recently 🤣 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 😅 |
|
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 😎 Cool that you can write a barebones nimib in like 100 lines of code 🤯 I think I understand what you mean by doing the capturing in the NbDoc now 👍 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:
nbEndCapturecould be hidden away in a template (maybe directly in 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 :)
Yeah, I ran into the word at the end and love it already 🥰. Actually I think I will use
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
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 |
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
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.
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
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? 🤣 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. |
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
Haha true, maybe I should start in that end 🤣 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. |
Update allblocks.nim in docsrc to document the new block! |
This is my attempt at implementing container blocks (#117) thus far.
TODO:
nbCollectAllNbJsto work with arbitrary numbers of nested levels of blocks. (recursive)nimibCodewithnbCodein docs.