Skip to content

Failing test with nasty HRs#56

Open
Soreine wants to merge 1 commit intojxson:masterfrom
Soreine:failing-test-with-hrs
Open

Failing test with nasty HRs#56
Soreine wants to merge 1 commit intojxson:masterfrom
Soreine:failing-test-with-hrs

Conversation

@Soreine
Copy link

@Soreine Soreine commented Aug 31, 2018

Hello, I'm opening this PR with a failing test, that yields the following result:

{ 
 attributes: '___',
 body: '----\n\nSome markdown following a 4 dashes horizontal rule\n\n\nSome more Markdown\n',
 frontmatter: '# Heading 1 followed by 3 _underscores_\n\n___'
}

for the following string:

----

Some markdown following a 4 dashes horizontal rule

---

# Heading 1 followed by 3 _underscores_

___

---

Some more Markdown

Here is what seems to be wrong about it:

  • I would expect attributes to always be a JSON, not a string.
  • I did not know a frontmatter could be elsewhere than the start of a document. Is there a way for me to specify that I want to ignore frontmatters that are not at the start of a document?

I don't know if this is an issue with the regex of front-matter to detect where a frontmatter is in the text, or if it is an issue with js-yaml for returning something that is not a JSON. Any help is appreciated.

@Soreine
Copy link
Author

Soreine commented Oct 22, 2018

Even if this PR is not merged in the end, any advice that could improve my understanding of YAML frontmatters would be greatly appreciated

@jxson
Copy link
Owner

jxson commented Nov 5, 2018

Sorry for the delayed response, I have been really short on spare time.

I'll have to dig into this some as I would like to avoid having a broken test be in the source, i.e. I want to fix the bug ;)

It might be possible to check the value in front-matter where the YAML is parsed, e.g. check the type around line 41 in index.js. If the value being returned from the parse call is the wrong type it can be worked around but ideally there should be fix upstream to the js-yaml library.

@Soreine
Copy link
Author

Soreine commented Nov 6, 2018

I'll have to dig into this some as I would like to avoid having a broken test be in the source, i.e. I want to fix the bug ;)

Yes of course, the goal is to merge the test and a fix that goes with it :p

If the value being returned from the parse call is the wrong type it can be worked around but ideally there should be fix upstream to the js-yaml library.

I said I would expect attributes to always be an object, not a string. But the YAML specs seems to be a superset of JSON, and I think just a string is valid JSON. So maybe it's not js-yaml's responsibility to check for these cases.

Also, I said that I did not know a frontmatter could be elsewhere than the start of a document. But actually the YAML spec says that there can be as many --- delimited sections anywhere inside a document.

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