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

Added support for flashcards within Admonition style code blocks #885

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DanielChicoUGR
Copy link

@DanielChicoUGR DanielChicoUGR commented Feb 24, 2024

Description:

As described in issue #747 , support for flashcards within Admonition style code blocks has been added. The plugin is now able to detect cards inside code blocks with ad-* style indicator, among impklemented in ad-monition pluggin

image

image

image

image

image

Changes:

  • Modification of the parser function to skip those blocks of code that do not begin with: "```ad-" or "~~~ad-"

@ronzulu
Copy link
Collaborator

ronzulu commented Feb 25, 2024

Hi @DanielChicoUGR, nice one!

I haven't had a chance to test this, but a couple of comments in the meantime:

Please include new unit test cases within parser.test.ts

About terminology, IMHO saying "Added integration with the Admonition Pluggin" implies actual integration with that plug-in, which isn't accurate. I think "Added support for flashcards within Admonition style code blocks" is more accurate.

There is a chance that the enhancement could break existing notes. This would occur if someone had included within an admonition code block something that matched any of the recognized card formats. Something that they didn't want to be treated as a card, and up till now hasn't been, e.g. typedef std::vector<Board> t_bvector;

Perhaps we need to make this new behavior optional.

Cheers
Ronny

@DanielChicoUGR DanielChicoUGR changed the title Added integration with the Admonition Pluggin Added support for flashcards within Admonition style code blocks Feb 25, 2024
@DanielChicoUGR
Copy link
Author

kk, U right in both cases, i will work on this today and add some unit test too.

One Question only, is it possible to modify this pr or may i reopen other pr and close this?? (Noob doubts sry)

thanks for the time and attention, cheers Daniel

@ronzulu
Copy link
Collaborator

ronzulu commented Feb 27, 2024

Hi Daniel, looking forward to the update 😄

Do you have an edit button at the top of this page? I see the following:

image

Recently I was granted "collaborator" permissions, that's why I can edit someone else's PR.

I assume that before I became a collaborator that I would have been able to edit any PR that I opened, but don't explicitly remember.

If you can't edit, then I think it's better if you let me know what changes you want and I will make them (easier than closing this PR and opening another)

Cheers
Ronny

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