fix(renderer): occasional parts rendering, markdown types #85
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It seems like the markdown rendering plugins get confused by nested code fences so we replace embedded fences with ` ` `
Instead of disabling embedded markdown rendering, we can use a longer outer codefence. It did require regenerating all of the test data.
Instead of just using the filename extension, use the neovim filetype (with some specific overrides). Fall back to the filename extension. This means we render our markdown sections with `markdown` as the type instead of `md` and that enables rendering embedded codefences correctly. Hopefully the last fix on this :)
Seems like nightly returns text as the type for .txt files which breaks our tests
This was a subtle one and it only shows up when we're not collapsing events so it wasn't caught by the unit tests. I've since modified the unit tests to also run the replays without collapsing to catch bugs like this in the future. If the first version of a part didn't result in something being rendered, future updates to that part would also not render anything. This was particularly likely to happen with tools as the first version of the part is often `step-start`. Because we didn't think this was a new part, we called `_replace_part_in_buffer` but that exits quickly because the part hadn't been rendered anywhere. While we could try to call `_insert_part_to_buffer` in `_replace_part_in_buffer` if the part hadn't been rendered, that would miss some subtle cases around error handling so the correct fix is consider the part new in `on_part_updated` if we haven't rendered it before.
Owner
|
Perfect, It's now merged. Thanks a lot for both fixes in this PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resubmission of #83
This started out as a smaller bugfix but along the way I noticed that sometimes parts were not rendering, so that's the more important issue, from: 4317d0b:
Now, onto the issue that started this PR:
I noticed that embedded codefences were screwing up our rendering. At first, I thought we needed to escape/replace them but then I realized we could use a longer codefence for our outer wrapper. I also realized that we weren't setting the markdown types correctly (markdown should be
markdownnotmd). Setting the codefence tomarkdownenables render-markdown to render embedded codefences correctly.Before:
After (the codefence is normally hidden, I just have the cursor there to show the longer codefence and correct type)