fix(renderer): occasional parts rendering, markdown types#83
Closed
cameronr wants to merge 8 commits intosudo-tee:mainfrom
Closed
fix(renderer): occasional parts rendering, markdown types#83cameronr wants to merge 8 commits intosudo-tee:mainfrom
cameronr wants to merge 8 commits intosudo-tee:mainfrom
Conversation
It seems like the markdown rendering plugins get confused by nested code fences so we replace embedded fences with ` ` `
cameronr
force-pushed
the
fix/markdown-double-render
branch
from
be49c31 to
c1e625a
Compare
Instead of disabling embedded markdown rendering, we can use a longer outer codefence. It did require regenerating all of the test data.
cameronr
changed the title
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.
cameronr
changed the title
cameronr
changed the title
Collaborator
Author
|
@sudo-tee This one is probably a little urgent so would love a review when you have time. |
sudo-tee
approved these changes
Oct 29, 2025
Owner
|
I'm sorry I think I messed up the conflict resolution with the github UI. |
Collaborator
Author
|
no worries, i can take a look in a few hours. fwiw, the GH UI merges have let me down before so I've stopped doing merges there. also, if you'd rather i can just rebase my changes on main and resubmit the pr? |
Owner
Yes that would probably be easier to just rebase and drop the merge commit I did |
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
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.
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:
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 anew part, we called
_replace_part_in_bufferbut that exits quicklybecause the part hadn't been rendered anywhere.
While we could try to call
_insert_part_to_bufferin_replace_part_in_bufferif the part hadn't been rendered, that wouldmiss some subtle cases around error handling so the correct fix is
consider the part new in
on_part_updatedif we haven't rendered itbefore.
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)