Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't replace block IDs in codeblocks #596

Merged
merged 4 commits into from
Jun 11, 2024

Conversation

tyler-dot-earth
Copy link
Contributor

@tyler-dot-earth tyler-dot-earth commented May 30, 2024

Fixes #595 (which i reported)

The title of this PR is a little too narrow compared to what this PR actually does (preserves code blocks actually not quite, see #599), but I wanted to match the issue that I'd opened.

Took a quick stab at not replacing ^block-ids within markdown codeblocks.

Overview of the changes:

  • Added a new note: src/dg-testVault/015 Code blocks.md
  • Pulled the replacement operation out of createBlockIDs into its own file/function (replaceBlockIDs) so it could be tested independently of the compiler
  • Added a test for the new replaceBlockIDs fn

The actual mechanism for preserving block IDs within codeblocks is pretty simplistic:

  1. Extract and replace code blocks with placeholders: the text.replace function collects the code blocks into an array and replaces them with unique placeholders.
  2. Process text outside code blocks: the text is processed for block patterns as before.
  3. Reinsert code blocks: the placeholders are replaced back with the original code blocks from the array.

I probably didn't do something quite how you would have - let me know what the delta is and I'm happy to address it 🤠 For example: I suspect that a new snapshot is probably recommended, but I'd like some guidance on performing that new snapshot.

I'm very motivated to get this fixed as it is affecting my published notes (for example).

@oleeskild
Copy link
Owner

Thank you for fixing this! It looks good, this project is in dire need of more unit tests, so thanks for taking the time to write them. As you mentioned, it would be nice if you could generate the new snapshot. You do this by running npm run dev, which will create a new build of the plugin in the src/dg-testVault folder. Try opening that folder in obsidian as a new vault, and you should find a new command in the command palette named Digital Garden: Generate Garden Snapshot.
Running this should modify the src/test/snapshot/snapshot.md file, which you can then inspect with git diff to see if it looks right.

Let me know if something doesn't work, or you need any help.

```
This codeblock has a transclusion syntax in it.
Check it out:
<div class="transclusion internal-embed is-loaded"><a class="markdown-embed-link" href="/001-links/" aria-label="Open link"><svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="svg-icon lucide-link"><path d="M10 13a5 5 0 0 0 7.54.54l3-3a5 5 0 0 0-7.07-7.07l-1.72 1.71"></path><path d="M14 11a5 5 0 0 0-7.54-.54l-3 3a5 5 0 0 0 7.07 7.07l1.71-1.71"></path></svg></a><div class="markdown-embed">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just noting that this is actually evidence of a very similar bug: #599

@tyler-dot-earth
Copy link
Contributor Author

Thank you for fixing this! It looks good, this project is in dire need of more unit tests, so thanks for taking the time to write them. As you mentioned, it would be nice if you could generate the new snapshot. You do this by running npm run dev, which will create a new build of the plugin in the src/dg-testVault folder. Try opening that folder in obsidian as a new vault, and you should find a new command in the command palette named Digital Garden: Generate Garden Snapshot. Running this should modify the src/test/snapshot/snapshot.md file, which you can then inspect with git diff to see if it looks right.

Let me know if something doesn't work, or you need any help.

@oleeskild Snapshot updated 📸 I left a comment on its output in this PR - there's a related issue (but this PR still fixes the issue it originally set out to 👍)

@oleeskild oleeskild merged commit 94adcf7 into oleeskild:main Jun 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code blocks mangling code on output/publish
2 participants