Skip to content

Ignore inline keys when they appear in an inline codeblock. #2542

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ithinuel
Copy link

This example (Some inline::codeblock) renders broken.

This change attempts at fixing this.

@ithinuel ithinuel marked this pull request as draft March 13, 2025 18:40
@ithinuel ithinuel force-pushed the inline-field-and-inline-codeblock branch from 4ba0774 to 359c4b7 Compare March 13, 2025 19:45
@ithinuel ithinuel marked this pull request as ready for review March 13, 2025 19:46
@holroy
Copy link
Collaborator

holroy commented Mar 13, 2025

Before even considering to merge this, please clean up your code and remove unwanted console.log() statements.

@ithinuel ithinuel force-pushed the inline-field-and-inline-codeblock branch from 359c4b7 to 8bbb962 Compare March 14, 2025 06:01
@ithinuel
Copy link
Author

Woops, sorry about that. console.log have been removed.

@ithinuel ithinuel force-pushed the inline-field-and-inline-codeblock branch from 8bbb962 to 266a2c1 Compare March 14, 2025 06:02
@ithinuel
Copy link
Author

@holroy This PR should now be more comprehensive than the initial proposal.
Please let me know what you think about it and if I missed anything. I’m happy to learn!

@holroy
Copy link
Collaborator

holroy commented Mar 15, 2025

After looking a little closer into the actual change, I think we need to discuss this a little further. Your code ignores any escaped backticks, and it is used in a context where the function is indicating that we're only looking for the inline separator.

This means that this function is most likely also used when we're looking for separators in entire paragraphs being used as field definitions, and possibly other cases.

In other words, the scoping of this change is wrong, although the idea of ignoring inline keys could be pursued further. Where and how to it though needs to be discussed a little further.

@ithinuel
Copy link
Author

Hehe, that’s the kind of thing I expected to have missed.

I’m happy to relocate it to where it belong. Would you be able to advise on the proper location?

With regards to the backtick escaping, I tried to replicate what obsidian does in the markdown interpretation, i.e. escape backticks only outside codeblocks. So here we only ignore the backtick if it is found to be an "entry" to a code block.
I didn’t add specific tests for triple ` blocks. As far as I can see it is not causing issues in the code rendering.
I don’t use inline fields (yet) so I don’t know if I broke anything on that side either (hopefully not).

@holroy
Copy link
Collaborator

holroy commented Mar 15, 2025

I'm currently working on releasing a new version of dataview and having some issues with that, but in due time I'll come back to you hopefully. Not sure where and how to tackle this issue, but using findSeparator() just seems wrong.

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.

2 participants