pietroppeter / nimib

nimib 🐳 - nim 👑 driven ⛵ publishing ✍
https://pietroppeter.github.io/nimib/
MIT License
181 stars 10 forks source link

Complete Allblocks #203

Closed dlesnoff closed 1 year ago

dlesnoff commented 1 year ago

Fix #194. The nbFile and nbShow blocks are missing in the documentation but could be added after the PR. I am not sure of their usage and was not very creative for examples today.

I added a TOC to list all blocks at the top and give quick access to their documentation.

The whale image (chosen for some kind of consistency with the icon) is licensed under the Unsplash license.

The documentation now requires Nimpy to work. which makes nimpy a soft dependency.

I removed the blocks description from the README.md. The documentation link won't work until the docs are added to pietroppeter's blog.

Other partials (blocks too?) in renders.nim to add in allblocks.nim:

HugoGranstrom commented 1 year ago

nbText nbCode nbFile nbShow nbJsFromCode

All of these blocks should preferably be added, but that could be done in a follow-up PR by someone else as well :smile:

pietroppeter commented 1 year ago

All of these blocks should preferably be added

I would skip nbShow which is not a NbBlock (it is more a command like nbInit) and should be deprecated (it is equivalent to nbSave with --nbShow runtime option). The others yeah should be added there.

HugoGranstrom commented 1 year ago

@pietroppeter nbShow(df) 😉

dlesnoff commented 1 year ago

I merged main into my branch (integrating my 2 last PRs). I added nbText, nbCode, nbFile, nimibCode blocks. I replaced every markdown code with the nimibCode block. I removed references to nbPython code block and the nimpy dependency. Various fixes:

I resolved all the conversations, made the requested changes and I am ready for another review.

I am really happy of this change, a document for all the blocks is what I have been looking for since I discovered the package !

dlesnoff commented 1 year ago

I made the requested changes ;)

pietroppeter commented 1 year ago

Thanks looks very good! The link to see the all blocks preview is https://bc467ce645e09423fd70--nimib.netlify.app (the link we have is an absolute url - since it has to work also for readme - so it does not work). Very minor thing: you should use nimibCode also for nbCodeSkip

HugoGranstrom commented 1 year ago

I did not understand, do we really drop the nbShow block when there is a special usage in one other document ?

Ah right, forgot to answer this one, nbShow is actually a new command that "pretty prints" Nim objects that define a toHtml proc. So it should be documented. But it is confusing because in the past we had a nbShow command that you could replace nbSave with to open the page in a browser. But now you just use nbSave and pass --nbShow as a command line argument. The naming is a bit unfortunate but we have discussed renaming it to --nbOpen instead. You could leave the nbShow example to us if you want to :).

I am really happy of this change, a document for all the blocks is what I have been looking for since I discovered the package !

Me too! It's so easy to take documentation for granted when you are so invested in a project yourself, so it is very valuable to get feedback on how we can improve the nimib documentation :D

HugoGranstrom commented 1 year ago

Once you fix Pietro's minor thing I'm content with merging this

dlesnoff commented 1 year ago

The link to see the all blocks preview is https://bc467ce645e09423fd70--nimib.netlify.app/ (the link we have is an absolute url - since it has to work also for readme - so it does not work).

Sorry, I am not sure to understand this. Do you want me to change the link to the allblocks.nim document in the index.nim (and README.md) ? This link does not point to an allblocks.nim but to the README.md of my PR.

dlesnoff commented 1 year ago

you should use nimibCode also for nbCodeSkip

I fixed that! I am done with this PR. To summarize, we should after this:

pietroppeter commented 1 year ago

The link to see the all blocks preview is https://bc467ce645e09423fd70--nimib.netlify.app/ (the link we have is an absolute url - since it has to work also for readme - so it does not work).

Sorry, I am not sure to understand this. Do you want me to change the link to the allblocks.nim document in the index.nim (and README.md) ?

This link does not point to an allblocks.nim but to the README.md of my PR.

Yeah, I forgot the important detail: https://b41374816c22d6cd819a--nimib.netlify.app/allblocks.html

Once you go to the link to latest netlify preview (and I updated with latest now), you can add allblocks.html to see the preview of that page.

pietroppeter commented 1 year ago

you should use nimibCode also for nbCodeSkip

I fixed that! I am done with this PR.

Thanks a lot, will merge this now!

To summarize, we should after this:

👍 (nbJsFromCode has multiple variants, they are all already documented in interactivity.nim so not urgent)

  • [ ] Add the allblocks document to pietroppeter.github.io website?

This should already happen automatically once this is merged

  • [ ] Maybe make nimibCode separation between nimibCode rendered code block, the highlighted code and output of these code blocks more explicit? It would be cool to have the name of the files appearing in the code blocks (notebook.nim, notebook.html).

Yeah, we could customize partial of nimibCode for this document in order to have something like:

⬇️⬇️⬇️ source (Here the source code) ⬇️⬇️⬇️ output (Here the output)