NbBlock as a container

[pietroppeter]

currently you cannot nest block inside one another e.g. you cannot do stuff like:

nbCode:
   nbRawOutput: "<div>"
   echo "hello"
   nbRawOutput: "</div>

the above is not a great use case, a better use case would be to create html tabs:

nbTabs:
  tab("example 1"):
    nbText: "my example 1"
    nbCode: discard
  tab("example 2"):
    nbText: "my example 2"
    nbCode: discard
  tab("example 3"):
    nbText: "my example 3"
    nbCode: discard

and also nimislides has a similar use case when it does:

slide:
  slide:
    ...
  slide:
    ...

as a first thought there should be 3 changes to make:

• change block making it a variant type based on a isContainer field. If it is container you have fields blocks: seq[NbBlock], blk: NbBlock as in NbDoc
• need also to change nb object to track who is current container, either nb itself or some container block (so that the newNbBlock template can be called with correct container where to add the block)
• finally we will need to change rendering, which should be straightforward

it is possible that this does not break too much the api, so it could be considered for 0.3.x

[HugoGranstrom]

Sounds good, something I've noticed is that we will probably have to take extra care of stdout capturing. Right now when we create a new block there are multiple things being echo'd by the newBlock template which would get in the way of our capturing. Also if we do

nbCode:
  nbCode:
    echo "Hello world"
  echo "Hello world again"

Which nbCode should capture the innermost "Hello world"?

[pietroppeter]

yes, that would be a possible issue (and the advice might just be: don't do that 😛), although in principle the current template captureStdout that we use might just work for that (it takes current stdout file pointer, creates a new file pointer when capturing then it puts it back).

[HugoGranstrom]

Haha ok then 🤣
Ohh 😮 yes that might actually work, but we would have to do something about the echos when creating a block as they will be captured by nbCode no matter how we do it. (Unless we disarm it around the echos specifically somehow without ignoring the bodies stdout)

[pietroppeter]

we would have to do something about the echos when creating a block as they will be captured by nbCode no matter how we do it.

ah right. in principle they could be not echos but something that is based on a stdout tracked initially... but honestly I do think that (at least in the beginning) we might just discourage people of nesting inside nbCode (or in general when captureStdout is involved).

[HugoGranstrom]

Now that I think of it we really don't want to nest things in nbCode either way as the output of that block has to be placed somewhere and I'm not entirely sure where it would be, it could very well be that it screws up the code block or something 😅 so yeah that's not a problem. It would be nice to be able to show how to use nimib in nimib without writing the code as strings but we have the "wrap it in a template and nbCode the template definition" for that 😎

[HugoGranstrom]

We could even automate that process with a specific nbNimibCode:

template nbNimibCode(body: untyped) =
  nbCode:
    template temp() {.gensym.} =
      body
  # Highjack the nbCode block:
  nb.blk.command = "nbNimibCode"
  temp()

and then add a step in the rendering plan of nbNimibCode which removes the first template line and unindents the code one step so the final code shown is just body.

This would allow us to do:

nbNimibCode:
  nbText: "Hello world"

without any problems with capturing stdout.

[pietroppeter]

from dicussion in #74 there might be the option of making every block a container block...

[HugoGranstrom]

So, my wishlist for working with container blocks is to be able to do two things:

1. Inspect the list of blocks contained in the block, with the ability to modify them.
2. If I don't specify anything else, the contained blocks should be available in a partial that I can add to the partial of my block. If you want to make your own custom logic you just don't use that partial.

[HugoGranstrom]

I have thought a bit about the API and one thing that is problematic is when we use newNbBlock, we pass in a blockImpl. And the body is run inside the blockImpl, so we can't just wrap the blockImpl in a "container" as we wouldn't be able to access global blocks anymore in blockImpl. So we want the body to be in the "container environment" but the rest of the blockImpl to be in the current "environment".

So how can we solve this? One idea is to expose a nbContainer template that the blockImpl has to wrap the body in and handles all the nb.blk and nb.blocks assignments for us:

newNbBlock(....):
  nbContainer:
    # code here is run in a new "environment"
    body
  # nb.blk is reset to the current block by nbContainer
  echo nb.blk.blocks # the contained blocks can be accessed here. 

Another problem is if we go the route of reassigning nb to be the current container, blocks wouldn't be able to access the fields of the top-level NbDoc anymore. This is relevant for nbCode as it accesses and modifies nb.sourceFiles when reading code from source. Instead, I think we should just keep the same NbDoc object all the time, but only switch out nb.blocks and nb.blk and then switch back to the parent's list and block when the environment has reached its end.

This is mainly some brainstorming from my side. Probably a lot of things I haven't thought about.

[pietroppeter]

My rough idea is that there is a stack of blocks and on top of the stack the current container. Every time you create a new block current block goes on top of the stack for the duration of block implementation. In principle nothing prevents the global nb to be always accessible. Of course many pieces should be adopted including code in newNbBlock.

Having said that I have tried to materialize this idea in a specific api and type implementation but have not yet succeeded. I have not tried really hard though, and my feeling (maybe hope) is that there is hidden (maybe even not so hidden) an easy way to do that...

[HugoGranstrom]

Yes it will become a stack, but I don't think we explicitly have to create the stack data structure. It is enough if we just keep track of the parent every time we create a new container. By adding a blk.blocks field to NbBlock, this is how simple the implementation of nbContainer would be and it would only require to take the blocks field into account in the rendering (nbNewBlock would be unchanged):

template nbContainer(body: untyped) =
  # This is the "stack" part where we save the current value
  # to be able to reset them when we reach the end of the
  # container and "pop" the stack:
  let currentBlk = nb.blk
  let currentBlocks = nb.blocks
  nb.blocks = @[]

  # run the containing blocks
  body

  # reset values and assign the blocks to the container block
  nb.blk = currentBlk
  nb.blk.blocks = nb.blocks
  nb.blocks = currentBlocks

I haven't actually tried this but it should work 🙃 And we would need a better name for the template, nbContainer sounds like it is a block itself which it isn't.

[pietroppeter]

That's an interesting solution with minimal changes to existing types! By using a template you let the template create all the temporary variables for the stack without the need to manage them. I was not thinking about a template because I guess this is something we should only use once in newNbBlock and it is normally never used by the user right? But the template itself is reused many times... so that is clever!

The only slight fear I have is that it might be more difficult to find out bugs if there are compiler related issues with templates (we do end up with quite a number of templates one inside the other and I do feel there are some bugs lurking there that I already ran into without quite understanding). This does not mean we cannot proceed like this, fearlessly challenging the compiler (and maybe later we might want to retreat to a safer and more explicit api...). So let's try!

As for the name, even more so if it is not something that we can reuse (although we might still want to make it public), we can go with a long descriptive one such as nbUseCurrentBlockAsContainer or similar

[pietroppeter]

Btw unrelated to the container thing but since we are messing up with newNbBlock, maybe there is also a way to check if the current containing block is capturing stdOut, putting it on pause and starting it back again later... in that way we might have something that helps us solve the issue with nbCode inside other templates or this is also something that would make captured text and shown images and tables appear in the correct order. Not that I think it is a very big issue if we have all captured text before or after. Also now that I think of this, it kind of means we need to manage a sequence of outputs, so mmmh no, not really convinced that this could work, so you can discard this remark entirely (unless it gives you better ideas!).

[HugoGranstrom]

I was not thinking about a template because I guess this is something we should only use once in newNbBlock and it is normally never used by the user right?

Exactly, it's only block-creators who will need to use this. And the nice thing is also that this change wouldn't break current code, as without using this template you just get the old behavior.

The only slight fear I have is that it might be more difficult to find out bugs if there are compiler related issues with templates (we do end up with quite a number of templates one inside the other and I do feel there are some bugs lurking there that I already ran into without quite understanding). This does not mean we cannot proceed like this, fearlessly challenging the compiler (and maybe later we might want to retreat to a safer and more explicit api...). So let's try!

I understand your fear, but as you say, we are already nesting templates inside templates inside templates so adding another layer doesn't hurt 🤣 Do you have any concrete examples of bugs you couldn't understand related to templates?

As for the name, even more so if it is not something that we can reuse (although we might still want to make it public), we can go with a long descriptive one such as nbUseCurrentBlockAsContainer or similar

Yes it needs to be public for library creators to access it (nimiSlides will need to use it for slide for example). nbUseCurrentBlockAsContainer sounds good to me 👍

Btw unrelated to the container thing but since we are messing up with newNbBlock, maybe there is also a way to check if the current containing block is capturing stdOut, putting it on pause and starting it back again later... in that way we might have something that helps us solve the issue with nbCode inside other templates or this is also something that would make captured text and shown images and tables appear in the correct order. Not that I think it is a very big issue if we have all captured text before or after. Also now that I think of this, it kind of means we need to manage a sequence of outputs, so mmmh no, not really convinced that this could work, so you can discard this remark entirely (unless it gives you better ideas!).

I think we can use the same concept of a stack for stdout as well. Right now we unmount the ordinary stdout and attach our own file. But what if we introduced a new nb.stdout field that at the top-level is the real stdout but as we go deeper we just stack our custom files. This would only require us to change captureStdout in a similar way to nbContainer. So in the case of a nbCode inside another nbCode:

1. The outermost nbCode takes over from the real stdout.
2. The inner nbCode takes over from the outer one and returns control to it when finished.

This way, the innermost block that uses captureStdout will be the one that captures the stdout, which I think is a reasonable behavior.

Furthermore, I think we should implement a nbDisableCaptureStdout template that disables any capture of stdout of its body. Wrapping the echos and stdout.writes in newNbBlock in this template would prevent these messages from being captured by its parent container.

So I don't think this is a bad idea, it's a good one ;) As for the placement of the output of the children, that's why we will introduce the > blocks partial so each block can decide for itself if it want its underlying blocks to be rendered before or after. Does this make any sense?

[HugoGranstrom]

I have experimented with this locally today and it seems to work. This is how an updated nbCode would look like:

template nbCode*(body: untyped) =
  newNbCodeBlock("nbCode", body):
    captureStdout(nb.blk.output):
      nbUseCurrentBlockAsContainer:
        body

nb.partials["containedBlocks"] = """
{{#blocks}}
{{&.}}
{{/blocks}}
"""

nb.partials["nbCode"] = """
{{>nbCodeSource}}
{{>nbCodeOutput}}
{{>containedBlocks}}
"""

nb.renderPlans["nbCode"] = @["highlightCode", "renderContainer"]

where renderContainer is a render proc which just renders the contained blocks and assigns them to context["blocks"]:

proc renderContainer(doc: var NbDoc, blk: var NbBlock) =
  var children: seq[string]
  for child in blk.blocks.mitems: 
    children.add doc.render(child)
  blk.context["blocks"] = children

The stdout handling was a lot trickier, mostly because of me not understanding the low-level file API. But I think I got something working in the end. Before creating a PR and testing further, is there something in the example above that you have any comments on?

[pietroppeter]

Looks good to me!

Do you have any concrete examples of bugs you couldn't understand related to templates?

There was a case when preparing slides for nimconf but forgot exactly what. There might be a remark in the comments...

[HugoGranstrom]

Ok, I will try to create a PR tomorrow then 👍

I tried looking through the NimConf files but I couldn't find any comments on this in the most recent files, and I didn't find any commit messages regarding it. So either I'm bad at reading or the comments are in an older commit. The only issue I can remember was the one with nbCode from different files, but that should be solved now when we look for the source in the correct files.

[pietroppeter]

another use case fo containers: nbFlex