Skip to content

Conversation

@timotheecour
Copy link
Member

@timotheecour timotheecour commented Mar 7, 2021

This PR fixes all such issues with runnableExamples by avoiding the bugs of renderModule, but underlying problem in renderModule (and repr) remains and will need to be fixed; at least it won't affect runnableExamples anymore; these are tracked in #17292.

future work

  • remove useRenderModule dead code
  • remove renderModule or add a test

@timotheecour timotheecour changed the title fix #13491 runnableExamples rendering fix #13491 #17279 runnableExamples rendering Mar 7, 2021
@timotheecour timotheecour changed the title fix #13491 #17279 runnableExamples rendering fix #13491 #17279 runnableExamples are now verbatim Mar 7, 2021
@timotheecour timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Mar 7, 2021
@timotheecour timotheecour marked this pull request as ready for review March 7, 2021 08:55
@timotheecour timotheecour changed the title fix #13491 #17279 runnableExamples are now verbatim fix #13491 #17279 runnableExamples now don't get lost in translation Mar 7, 2021
@timotheecour timotheecour requested a review from ringabout March 7, 2021 08:56
@timotheecour timotheecour removed the TODO: followup needed remove tag once fixed or tracked elsewhere label Mar 7, 2021
rdoccmd = n1.strVal

var docComment = newTree(nkCommentStmt)
let useRenderModule = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So ... there is a bunch of dead code here regarding useRenderModule. Why?

Copy link
Member Author

@timotheecour timotheecour Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would snowball into making renderModule itself deadcode, which opens another can of worms.
How about i defer to another PR removing this deadcode, and decide there whether to also remove renderModule or add some test for it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, ok.

@timotheecour timotheecour added the TODO: followup needed remove tag once fixed or tracked elsewhere label Mar 9, 2021
@Araq Araq merged commit d161d27 into nim-lang:devel Mar 9, 2021
@timotheecour timotheecour deleted the pr_fix_13491_runnableExamples_invalid branch March 9, 2021 07:28
ringabout pushed a commit to ringabout/Nim that referenced this pull request Mar 22, 2021
… in translation (nim-lang#17282)

* fix nim-lang#13491 runnableExamples rendering
* fix a runnableExamples thanks to this bugfix
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
… in translation (nim-lang#17282)

* fix nim-lang#13491 runnableExamples rendering
* fix a runnableExamples thanks to this bugfix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

TODO: followup needed remove tag once fixed or tracked elsewhere

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runnableExamples gives wrong values for number literals runnableExamples transforms do: => : and (expr) => expr, producing invalid code

2 participants