Can't use nbCode inside a template

[HugoGranstrom]

Using the now default codeAsInSource has the following problem (codeFromAst doesn't have this problem):

nbCode (or any block reading code) can't be used inside another template. This is due to the fact that here, the code is looking for "nbCode" to start the command before the code block. But if we for example have:

template notNbCode(body: untyped) =
  nbCode:
    body

notNbCode:
  echo "Hello world"

Then, because the block that is created has command "nbCode", it will not find notNbCode to match it and will continue upwards to look for the start of the code block. And it will only find it when it reaches inside the definition of the notNbCode template.

Expected output

echo "Hello world"

Actual output

body

bCode:
ho "Hello world"

Other notes

template nbCodeInBlock*(body: untyped): untyped =
  block:
    nbCode:
      body

This only works because nbCodeInBlock happens to start with nbCode...

Possible solutions

• Change the criterion for finding the startLine of the code block to use something else than the command string. The problem is how to reliably do that.
• Implement the template as a block, borrowing renderplan and partial from nbCode

template notNbCode(body: untyped) =
  newNbCodeBlock("notNbCode", body):
    captureStdout(nb.blk.output):
      body

nb.renderPlans["notNbCode"] = nb.renderPlans["nbCode"]
nb.partials["notNbCode"] = nb.partials["nbCode"]

notNbCode:
  echo "Hello world"

[pietroppeter]

Thanks for the detailed write up (you might want adding a mention adding that this is an issue with - now default - CodeAsInSource and it disappears when using CodeFromAst).

This is an issue that will not easily go away until a better way to do CodeAsInSource is available (if it is indeed possible).

Situation is not too bad though, since I guess the request of using a newNbCodeBlock is not too heavy. And for a quick hack, I think the possibility of having this work by starting the new command with nbCode (as in nbCodeInBlock) is something that we might want to continue support in the meantime.

The important thing is to be aware of this (I did not know nbCodeInBlock worked by "chance"). This issue is a good first step, I plan to mention this in nimconf presentation and probably it might be worth to have a short section on limits of codeAsInSource somewhere in the documentation.

[HugoGranstrom]

Thanks for the detailed write up (you might want adding a mention adding that this is an issue with - now default - CodeAsInSource and it disappears when using CodeFromAst).

Good point 👍

This is an issue that will not easily go away until a better way to do CodeAsInSource is available (if it is indeed possible).

I can look into this, but it won't get a very high priority until after NimConf.

Situation is not too bad though, since I guess the request of using a newNbCodeBlock is not too heavy. And for a quick hack, I think the possibility of having this work by starting the new command with nbCode (as in nbCodeInBlock) is something that we might want to continue support in the meantime.

The problems start when you want to use a template to combine multiple blocks and have to create a new custom block that combines their renderPlans and partials. That is a bit cumbersome. But for single blocks, it's not too bad.

The important thing is to be aware of this (I did not know nbCodeInBlock worked by "chance"). This issue is a good first step, I plan to mention this in nimconf presentation and probably it might be worth to have a short section on limits of codeAsInSource somewhere in the documentation.

Indeed. A new document named caveats or limitations does sound like a good idea :D

[HugoGranstrom]

I haven't done anything regarding this yet, but I have a plan. But it is a cumbersome plan... We basically have to create our own parser to find the line of the "command" without knowing which command it is. And to recap, the reason we can't just use the line info we get from the compiler: it only gives us the position of the first symbol/ident, e.g.:

nbCode:
  # This is a comment
  let
    a = 1
    b = 2

It is the line a = 1 that the compiler will give us. So we manually will have to look for these kinds of "invisible" structures which aren't recognized by the line info. And that is my plan for now, to find as many of them as possible. Then when that is done, the algorithm would be something like this:

1. While we have "invisible structures", step back.
2. When no "invisible structures" are left, we should be back at the "command".

I will create a post below this one where I will update all corner cases as I find them.

[HugoGranstrom]

Corner cases

Let/var statements

let a = 1
#   ^ here

var
  a = 1
# ^

Comments

# this line will not be found

Block

I think this one didn't work either, will have to check it again.

block:
  foo()
# ^

[pietroppeter]

Looks good that you are planning to pick up this :) do not have much to add, expect maybe you could see if it makes sense to write a nimib document with the examples (with a special template that shows where it would put the lineinfo?). Another consideration is: this "wrong" line info is it something that could be considered a bug in Nim compiler and the better place to fix it is there? Not holding my breath there, I suppose there might be a simple explanation why Nim compiler should not bother.

[HugoGranstrom]

ehm.... It seems like we just have a bug in startPos. lineInfoObj() seems to work correctly for all the cases I listed (var/let and blocks) except regular comments, it's just that the logic is wrong, so we look at their children's lineinfo instead 🙈 So it turns out the compiler has been doing the right thing the whole time, it's just us who has been handling it wrong.

I think this logic was there already when I got the code, but I didn't understand it, so I didn't dare to change it 🤣
But now I propose we change the logic there so we get the lineinfo directly by default. And if we find a node kind which doesn't work, we add it as a special case. (compared to now where we have an, to me, arbitrary selection of kinds in the case statement). Then we only have to take comments into account which simplifies things dramatically.

[pietroppeter]

well that's... good news? :)

[HugoGranstrom]

well that's... good news? :)

It's great news! :D Now we can reliably get the start of a code block (line and column), so this simplifies this process a lot. I still have to implement it though and test so it doesn't break anything accidentaly 😅

Another thing I've thought about, what do we expect codeAsInSource to output in this case:

template foo(body: untyped) =
  nbCode:
    echo "Hello world"
    body

foo:
  echo "Bye world"

Just by looking at the source file we can't know the code actually is:

echo "Hello world"
echo "Bye world"

So we will get:

echo "Hello world"
body

But then this isn't perhaps a case that's too normal to stumble upon 🤷

[HugoGranstrom]

Partially fixed by #163

The cases that still don't work is when code from different places are combined, like here:

template foo(body: untyped) =
  nbCode:
    body # This code is from line 7
    echo "Hello world" # this code is from line 4

foo:
  echo "Goodbye world"

[HugoGranstrom]

I do have ideas for how one could solve this though. But it is complicated and I'm not tempted to try it for a fair while. But the gist of it is this:

1. Loop over each child of the nnkStmtList and record its start/end position.
2. If two adjacent children also are located on adjacent lines, assume they are in the same code block.
3. If they are not on adjacent lines, assume they are in different code blocks.
4. Process each code block obtained separately and glue them together in the end.

Step 1 feels pretty easy. Step 2 is where we are stepping into trouble, because two adjacent lines might have an empty line in between them, but that line might also be something else. Even worse if there is say, 3 empty lines instead. Then they might actually be from different code blocks. Step 4 should be pretty straight forward as well if we align each block separately first.