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 #30, bug where the package cannot properly parse a file that contains Markdown code blocks with front matter in them #38

Merged
merged 12 commits into from
Apr 6, 2022

Conversation

caendesilva
Copy link
Contributor

This PR fixes #30

The PR is fully backwards compatible as it does not change the existing parser, but instead adds a new one: YamlFrontMatter::markdownCompatibleParse() which uses code by https://github.com/eklausme (I hope this is okay, I'm happy to add you as a co-author if you see this!

Before:
image

After:
image

The source markdown used for the example: (with the inline backticks replaced)

---
title: test
foo: bar
---

A paragraph in a Markdown Post.
This file contains a Markdown code block that the original parser does not handle.
See https://github.com/spatie/yaml-front-matter/discussions/30.

'''
---
title: The Title of Markdown Code
---

A paragraph in Markdown Code
'''

src/YamlFrontMatter.php Show resolved Hide resolved
@caendesilva
Copy link
Contributor Author

Okay I refactored the code to use an improved version of what I used for the Hyde framework. I'm converting everything to arrays, which may be a bit inefficient but I think it makes it much more readable and understandable. I also added two more tests. Let me know if its okay or if I should change anything else.

This is my first community PR so I'd love to get feedback for the many more I hope to make :)

src/YamlFrontMatter.php Outdated Show resolved Hide resolved
src/YamlFrontMatter.php Outdated Show resolved Hide resolved
src/YamlFrontMatter.php Show resolved Hide resolved
@caendesilva
Copy link
Contributor Author

Let me know if there is anything else I can improve!

Also, @freekmurze, I really want to thank you for taking the time to give me such great feedback and helping me become a better developer! This is my first time contributing to a community package and thanks to you I feel like I have learned a lot!

src/ComplexMarkdownParser.php Outdated Show resolved Hide resolved
src/ComplexMarkdownParser.php Show resolved Hide resolved
src/ComplexMarkdownParser.php Show resolved Hide resolved
src/ComplexMarkdownParser.php Outdated Show resolved Hide resolved
src/ComplexMarkdownParser.php Outdated Show resolved Hide resolved
src/ComplexMarkdownParser.php Show resolved Hide resolved
src/ComplexMarkdownParser.php Show resolved Hide resolved
src/ComplexMarkdownParser.php Show resolved Hide resolved
src/ComplexMarkdownParser.php Show resolved Hide resolved
@caendesilva
Copy link
Contributor Author

Okay, pushing the commits now. I made it all in separate commits as I think that makes it easier to follow. I assume you will probably squash them before merging. As you can see I did not change the double quotes to single quotes as I wanted to await your input on the concern I placed as a comment in the first change conversation.

@freekmurze freekmurze merged commit f49f228 into spatie:main Apr 6, 2022
@freekmurze
Copy link
Member

Thank you!

@caendesilva
Copy link
Contributor Author

That's super cool! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants