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

Undocumented change in YAML metadata parsing (Pandoc 2.2.3) #4819

Closed
cagix opened this issue Aug 7, 2018 · 10 comments
Closed

Undocumented change in YAML metadata parsing (Pandoc 2.2.3) #4819

cagix opened this issue Aug 7, 2018 · 10 comments

Comments

@cagix
Copy link
Contributor

cagix commented Aug 7, 2018

Pandoc 2.2.3 and 2.2.3.1 read

---
foo: 42
...

as

Pandoc (Meta {unMeta = fromList [("foo",MetaInlines [Str "42"])]}) []

However, in pandoc 2.2.2 and earlier, the same YAML would produce a MetaString instead:

Pandoc (Meta {unMeta = fromList [("foo",MetaString "42")]}) []

Is this change of behaviour intended? Since it may break some Lua filters, it should be documented.

@cagix
Copy link
Contributor Author

cagix commented Aug 7, 2018

running the Linux binaries under Debian Stretch

cagix added a commit to cagix/pandoc-lecture that referenced this issue Aug 7, 2018
In Pandoc 2.2.2 and earlier, a YAML element
---
foo: 42
...
would produce a `MetaString`.

Since Pandoc 2.2.3 this will result in a `MetaInlines`.

(see jgm/pandoc#4819)
@msprev
Copy link

msprev commented Aug 7, 2018

Another undocumented change in pandoc 2.2.3 and 2.2.3.1:

---
foo: true
...

now produces

Pandoc (Meta {unMeta = fromList [("foo",MetaInlines [Str "true"])]}) []

but in earlier versions this produced a MetaBool:

Pandoc (Meta {unMeta = fromList [("foo",MetaBool True)]}) []

This has currently broken panzer.

@jgm
Copy link
Owner

jgm commented Aug 7, 2018

These changes weren't intentional. It appears that my changes to YAML parsing in 2.2.3, which were intended to fix one regression, caused several other ones. Clearly we need more extensive tests!

Have you verified that these cases have the old behavior with version 2.2.2.1?

@jgm
Copy link
Owner

jgm commented Aug 7, 2018

Actually I don't see why '42' should be parsed as a MetaString. MetaInlines seems better and more consistent here (even though that is a change). However, we should keep parsing of 'true' as a boolean, since we actually have a boolean type constructor in MetaValue.

@msprev
Copy link

msprev commented Aug 7, 2018

I've verified the boolean case has the old behaviour with version 2.2.2.1. This version of pandoc does not generate any problems for me.

I agree that '42' should be parsed as a MetaInlines. Note that under 2.2.2.1:

---
foo: 42
...

produces:

Pandoc (Meta {unMeta = fromList [("foo",MetaString "42")]}) []

while:

---
foo: "42"
...

produces:

Pandoc (Meta {unMeta = fromList [("foo",MetaInlines [Str "42"])]}) []

which seems wrong...?

@cagix
Copy link
Contributor Author

cagix commented Aug 7, 2018

Have you verified that these cases have the old behavior with version 2.2.2.1?

@jgm

  • 2.2.2: Pandoc (Meta {unMeta = fromList [("foo",MetaString "42")]}) []
  • 2.2.2.1: Pandoc (Meta {unMeta = fromList [("foo",MetaString "42")]}) []
  • 2.2.3: Pandoc (Meta {unMeta = fromList [("foo",MetaInlines [Str "42"])]}) []
  • 2.2.3.1: Pandoc (Meta {unMeta = fromList [("foo",MetaInlines [Str "42"])]}) []

so this change was introduced in 2.2.3 ...

@cagix
Copy link
Contributor Author

cagix commented Aug 7, 2018

why '42' should be parsed as a MetaString. MetaInlines seems better and more consistent here (even though that is a change)

Well, in both cases a single number ends up as string somehow: in 2.2.2 directly as MetaString, in 2.2.3 as Str within MetaInlines ...

For me, both interpretations can work (although I'd vote to read/represent a number actually as a number). However, since this new behaviour does break things, it should be documented ...

@cagix
Copy link
Contributor Author

cagix commented Aug 7, 2018

@msprev confirmed.

in 2.2.2.1 foo: 42 is read as MetaString "42", whereas foo: "42" is read as MetaInlines [Str "42"]).

2.2.3.1 yields for both YAML versions the same representation MetaInlines [Str "42"]).

@jgm jgm closed this as completed in b76203c Aug 7, 2018
@jgm
Copy link
Owner

jgm commented Aug 7, 2018

Since we don't have a numerical type in MetaValue (perhaps that was a mistake), we have to represent numbers as strings, so I'm going with MetaInlines. I document the change in the changelog for 2.2.3.2...

@cagix
Copy link
Contributor Author

cagix commented Aug 7, 2018

@jgm thanks for the quick fix :)

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

No branches or pull requests

3 participants