Allow embedding a nim file as an nbCode block

[PhilippMDoerner]

This came to me while writing on the owlkettle nimibook.

Owlkettle has a bunch of example.nim files for example applications for individual widgets.
I would love to be able to embed those inside of the owlkettle nimibook instead of linking to the files.

The idea would be something like:

import nimib, nimibook

nbInit(theme = useNimibook)

nbText: """
## Example
Look at this very cool example of a scale widget
"""

nbCode(file = "owlkettle/examples/widget/scale.nim")

nbText: """
As you can see it does wonderful things etc. etc.
"""
...

This is just an example to demonstrate what I mean, what the Syntax for it looks like exactly isn't all that important.

[HugoGranstrom]

Would you like to just show the code or to run it as well?

[PhilippMDoerner]

For my specific usecase I only need to show the code, as another nimble command will compile all the individual examples.

For the feature generally I think it makes sense to compile and run these files same as normal nbCode blocks.
An nbCode block gives me the promise normally that the code inside it can definitely compile and run.
That is a promise that should be upheld by default imo, anything else breaks what I expect off of an nbCode block.

Could be of course nice if I could pass a boolean that allows me to say "don't compile the file, just show" or enable passing specific compilation flags for the compilation of this file specifically, but I don't think that's particularly necessary for a first stab at the feature.

[HugoGranstrom]

Just showing the file should be easy, but running it as well sounds a bit harder, especially if we want to capture the output of the external program. This would be a separate thing from nbCode though, as this is a totally different thing. Maybe nbCodeFile or nbReadFile or nbRunFile or something else. nbFile already does something similar, but it also writes a file, which isn't what we want in this case.

It should be totally possible, though, it's just a matter of deciding on the details of how it should work. It should be implemented in nimib itself though.

[PhilippMDoerner]

Just in case you want feedback on that nbNimFile and nbReadFile seem sensible:

• nbNimFile: Reads in a nim file and compiles it
• nbReadFile: Reads in any file (event a nim file) and displays its contents

Though the other options also look perfectly fine.
How is this to progress from here?
Await feedback from pietroppeter and based on that somebody (me? you? peter?) opens up an issue in nimib?

[HugoGranstrom]

Thinking a bit more about this, we could add an overload for nbFile that only takes the filename (Instead of filename and content). Then we don't have to create a separate nbReadFile. The one which runs the external code as well would need to be separate, though.

How is this to progress from here?
Await feedback from pietroppeter and based on that somebody (me? you? peter?) opens up an issue in nimib?

Apart from the names of the functions, I would that we are ready to start working on this. Feedback can always come later be adapted into the work. Do you feel tempted to implement this in nimib yourself? With guidance of course if you need it 😄

[PhilippMDoerner]

Do you feel tempted to implement this in nimib yourself? With guidance of course if you need it 😄

Not really and I don't see myself having any in the near future.

I'm currently like 4 PRs deep in owlkettle, 2 more in the pipeline once a blocking one is merged and 3 more issues I want to tackle.
That is before adding more examples for the individual widgets there and making myself a list of widgets that are missing.

The entire thing dawned on me solely because I'm currently rather active throwing stuff at owlkettle and improving the entire doc situation there even further to make it as smooth an entry point as it can be.

[HugoGranstrom]

That's totally understandable 😁 I'll see if I manage to get any work done on this during the weekend 👍

[pietroppeter]

well, I would say thanks for the input @PhilippMDoerner, that's useful and it is perfectly fine to give feedback like this without any need of committing any work. We do ask for politeness though ;)

I agree with @HugoGranstrom that an overload for nbFile that implicitly reads instead of writing a file looks like a good idea.

[PhilippMDoerner]

Errrr was I impolite anywhere? That was not my intention, apologies if I formulated some piece of text badly anywhere. If I did please say so I'll try to keep more of an eye on it.

[HugoGranstrom]

Errrr was I impolite anywhere? That was not my intention, apologies if I formulated some piece of text badly anywhere. > If I did please say so I'll try to keep more of an eye on it.

I didn't perceive you in any bad way at all. I think he just said it as a general remark against the not-always-so-polite tone that some people have in parts of the community 🤷

[pietroppeter]

No no it was all fine. I just wanted to highlight the fact that feedback through issues is appreciated. Sorry if that sounded defensive in some way

[pietroppeter]

we do ask for politeness

Ah, I understood! It is us that we ask if someone is interested to submit a PR because we try to be polite. I was not asking for politeness. I guess I used a wrong English expression translating directly from Italian. Haha sorry for the misunderstanding. 😶‍🌫️

[PhilippMDoerner]

I did some staring at the code as I'm waiting for real life to give can.l some spare time to look at PRs 😄 .

Doesn't it basically suffice in nimib to have this?:

template nbFile*(name: string) =
  let fileContent = readFile(name)
  newNbSlimBlock("nbText"):
    nb.blk.output = text

Like I have no clue what any of this does bar the template call, but basically this just replicates the nbText template but fetches the contents of a file from a given string-filepath beforehand.

If I knew how in nimib I'd test it out, so I don't know if that actually works, but my best guess is that it should.
It shouldn't clash with the other nbFile templates either because:

1. it doesn't have a second parameter unlike template nbFile*(name: string, content: string) =
2. it doesn't have a body so it doesn't clash with template nbFile*(name: string, body: untyped) = . Or rather, it shouldn't have a body in my mind, since I imagine the syntax calling it would be nbFile("/some/path/blub.nim").

That seems pretty straightforward. So the necessary steps then would be:

• add these 3 lines to nimib
• add tests for those to nimib (? I don't quite know how or if that's even necessary/desired since there's no tests for any of the other nbFile blocks)
• upgrade the nimib version in nimibook
• do a version bump on nimibook
• No next step, we're done?

[pietroppeter]

yes, it should be something almost as simple as that. to have the same output of the other nbFile blocks it should be:

template nbFile*(name: string) =
  ## Read content of filename
  newNbSlimBlock("nbFile"):
    nb.blk.context["filename"] = name
    nb.blk.context["ext"] = name.getExt
    nb.blk.context["content"] = readFile(name)

I have adapted the code in https://github.com/pietroppeter/nimib/blob/dc890600a2274aaec8da71f5e16be084a7f15614/src/nimib.nim#L129C1-L137C1

The reason why we would use newNbSlimBlock("nbFile") instead of newNbSlimBlock("nbText") is that we want the content rendered differently, as in here: https://pietroppeter.github.io/nimib/allblocks.html#nbfile

The rendering code of nbFile (in the default html backend) for reference is this one: https://github.com/pietroppeter/nimib/blob/dc890600a2274aaec8da71f5e16be084a7f15614/src/nimib/renders.nim#L33

Indeed tests are desirable for nimib, let me quote the steps in nimib's CONTRIBUTING.md for adding a new block:

• add the logic in nimib.nim
• add the rendering in nimib\renders.nim
• add some test in tests (if it make sense)
• add an example in allblocks.nim
  • ⚠️ currently README.md depends on docsrc\index.nim, you will have to modify docsrc\index.nim and run nimble readme to update the readme. we will remove this dependency, see remove dependency of readme from index.nim nimib#141

• first step is the logic which are the 5 lines above
• rendering not needed since we are adding a new variant on an existing block
• tests as mentioned are desirable but they would have to be in tests/tnimib and we do not currently have test for nbFile so I would accept a PR without a test for this (we should probably open an issue that details all missing tests from tnimib...)
• instead you might want to add another example to allblocks.nim adding a small nim sample file (mmh, probably you can reused the example.nim that is written in the step before).
  • I am noticing now that apparently we have a files.nim document that is not linked elsewhere. this at some point should be cleaned up as the tests above but not necessarily in the same PR
• you should not have the issue of README.md since you do not have to touch it

Next steps are as you mention, actually we should just release a new nimib version and I would not bump the version in nimibook (this new block is not strictly required in nimibook). Instead in your own nimibook you will add the requirement with the new nimib version.

The release of the new nimib version would be up to us after your PR and we will follow the process outlined here: https://github.com/pietroppeter/nimib/blob/main/changelog.md (we should actually have a release since I just checked and the last one was in march and there have been a few contributions since then, it has been a messy period in the last few months...).

Thanks for pinging and giving us feedback on this! :)

[pietroppeter]

we do ask for politeness

Ah, I understood! It is us that we ask if someone is interested to submit a PR because we try to be polite. I was not asking for politeness. I guess I used a wrong English expression translating directly from Italian. Haha sorry for the misunderstanding. 😶‍🌫️

and I hope you saw that the "politeness" remark was not directed at you. It was totally a communication blunder on my side (see the reply here #85 (comment)) that I just quoted here above). You have been absolutely very polite and helpful all the way through, I am sorry if my mistake caused you some discomfort! 🙇

[PhilippMDoerner]

No worries regarding the politeness, I was just too lazy to reply after reading and acknowledging it 😄

Note that it appears impossible to run the docs task. It doesn't run even with happyx installed, neither after running nimble docsdeps.

[PhilippMDoerner]

With the nimib PR merged, it's basically only a matter of first nimib, then nimibook releasing a new version, would that be correct?

[HugoGranstrom]

Only a nimib release is necessary. Nimibook will use the version of nimib that is the most recent (I think).