Skip to content

Conversation

@christian-schulze
Copy link
Contributor

Resolves #67

Adds initial markdown support for Admonitions (using github syntax):

> **note** Note Admonition
> This is a Note Admonition

And:

> **warning** Warning Admonition
> This is a Warning Admonition

@zefhemel zefhemel self-requested a review December 11, 2022 10:44
Copy link
Collaborator

@zefhemel zefhemel left a comment

Choose a reason for hiding this comment

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

Nice start, have a look at my comments! Haven’t been able to actually test this yet (not behind a computer) but this should already be useful.

@christian-schulze
Copy link
Contributor Author

Cheers for the feedback @zefhemel. I've just pushed updates addressing these.

@zefhemel
Copy link
Collaborator

zefhemel commented Dec 11, 2022

Looks reasonable enough from the code perspective, I’ll check it out (literally) later to see if actually works 😉 before merging (QA you know). Or is there more you’d want to change (since it’s still marked as WIP).

@christian-schulze
Copy link
Contributor Author

christian-schulze commented Dec 11, 2022

Looks reasonable enough from the code perspective, I’ll check it out (literally) later to see if actually works wink before merging (QA you know). Or is there more you’d want to change (since it’s still marked as WIP).

I think there are many things that can be added, more types, different syntax support, custom types etc. But that can be for future PR's.
If you don't find any bugs am happy for you to merge. Will remove WIP.

@christian-schulze christian-schulze changed the title WIP - initial admonition support (github format) Initial Admonition support (github format) Dec 11, 2022
@zefhemel
Copy link
Collaborator

zefhemel commented Dec 11, 2022

Overall this works ok. The only issue I have is when the live preview kicks in. In its current behavior, this works strangely when adding spaces. I'm exegarating it in this video, but adding a space (even one) after **Note** is completely invisible and feels buggy:

CleanShot.2022-12-11.at.20.51.29.mp4

How I would solve this myself is to simply be more conservative when to actually show the live preview icon. I would simply not do any of that magic if the cursor is on the first line (the line with the admonition). That way you can always see all the markup (including spaces), and when you move to the second (non-header) line, the header renders as a preview. This also solves the "two stages of preview" problem where depending on where your cursor is it either shows **Note** or Note or the icon. What do you think?

Alternatively somehow the space thing needs to be handled differently, now it feels a bit broken. Come to think of it, perhaps the problem is simply in your regex? It may be addressed differently.

@christian-schulze
Copy link
Contributor Author

@zefhemel have addressed your other feedback.

Regarding the replacement of icon, have changed it to your first suggestion (within first line or not). Its a nifty feature but not essential. Will take another look at that later on.

@zefhemel
Copy link
Collaborator

Great, this is nice. I'll merge it, we can tweak things later. Thanks!

@zefhemel zefhemel merged commit fd16629 into silverbulletmd:main Dec 12, 2022
@christian-schulze christian-schulze deleted the admonition-support branch December 12, 2022 08:21
@christian-schulze
Copy link
Contributor Author

Great, this is nice. I'll merge it, we can tweak things later. Thanks!

Awesome thanks. You've got an interesting and useful project here, I'm happy to contribute more in future.

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.

Admonition support

2 participants