pietroppeter / nimib

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

Fix codeAsInSource #105

Closed HugoGranstrom closed 2 years ago

HugoGranstrom commented 2 years ago

Let's see if this works better now...

It took a day but I think I've found a somewhat non-chaotic implementation that seems to work. I added a few tests but I will add a few more and manually go through the docs to see if it looks OK.

fixes #97

TODO

HugoGranstrom commented 2 years ago

Ok, that seems to have fixed it but why did it work locally without any errors... 🤷

pietroppeter commented 2 years ago

Nice! Can you try also to remove the when defined(nimibCodeFromAst) in t nimib.nim?

HugoGranstrom commented 2 years ago

Something smells weird here, when the defined(nimibCodeFromAst) was included no errors. But when it was removed it gives an error at the very end (after the last test has been run with OK). I read somewhere that a plain Error: execution of an external program failed: means that the program returned a non-zero error code but I have idea how that could be dependent on this when-statement :/

HugoGranstrom commented 2 years ago

To make it even stranger, running it with nim r gives this error but doing `nim c and then running the executable doesn't give any errors

HugoGranstrom commented 2 years ago

Oh wait I'm just blind, there was a failed test 🤦

HugoGranstrom commented 2 years ago

OK seems like codeFromAst gives a different result than codeAsInSource for this one:

nbTextWithCode: """
hi
how are you?
"""

codeFromAst rewrites it as """hi removing the newline.

pietroppeter commented 2 years ago

ah, right, it is to be expected! In nim multiline string literal remove the first newline so CodeFromAst does not see it, while CodeAsInSource sees it as it is written. They are both right! Probably the easieast way to fix it would be a short template/proc used only in tests and added to the multiline strings involved here:

template normalizeMultilineStringLiteral(s: string) =
  when not defined(CodeFromAst):
    if s[0] == "\n":
      return s[1 ..< s.len]
pietroppeter commented 2 years ago

didn't see you already had the fix. fine as it is for me!

HugoGranstrom commented 2 years ago

ah, right, it is to be expected! In nim multiline string literal remove the first newline so CodeFromAst does not see it, while CodeAsInSource sees it as it is written. They are both right! Probably the easieast way to fix it would be a short template/proc used only in tests and added to the multiline strings involved here:

TIL :D

didn't see you already had the fix. fine as it is for me!

It is the only case where it matters thus far so making a template only feels warranted when we have a few test that needs it 👍

HugoGranstrom commented 2 years ago

I've gone over the docs now and to my eyes, it looks like it's working but I would appreciate a second opinion on that 😅

pietroppeter commented 2 years ago

I also give it a pass on all docs and do not find anything strange. We could at some point create a karax app that does a html diff between old and previous docs ;)

I noticed a couple of things while reading docs:

HugoGranstrom commented 2 years ago

I also give it a pass on all docs and do not find anything strange. We could at some point create a karax app that does a html diff between old and previous docs ;)

🙏 Haha 🤣😎

Fixed your remarks