preaction / Statocles

Static website CMS
http://preaction.me/statocles
Other
84 stars 33 forks source link

Frontmatter: bools aren't parsed as such #593

Open cxw42 opened 4 years ago

cxw42 commented 4 years ago

(At most tangentially related to https://github.com/preaction/Statocles/issues/541#issuecomment-329364711)

System:

$ statocles -v
Statocles version 0.095 (Perl v5.26.3)
Locale: en_US.UTF-8
$ uname -a
CYGWIN_NT-6.1 Desktop 3.0.7(0.338/5/3) 2019-04-30 18:08 x86_64 Cygwin

Input file:

---
disable_content_template: false
---
# Header

Expected: parses OK

Observed:

Error creating document in "index.markdown": Value "false" is not valid for attribute "disable_content_template" (expected "Bool")

It looks like this is because Statocles::Document uses YAML, not YAML::PP or YAML::XS. For example:

$ perl -MYAML -MData::Dumper -E 'say Dumper Load "foo: true"'
$VAR1 = {
      'foo' => 'true'     # <== NOTE: string
    };

$ perl -MYAML::PP=Load -MData::Dumper -E 'say Dumper Load "foo: true"'
$VAR1 = {
      'foo' => 1          # <== NOTE: boolish
    };

I looked in the test suite and didn't see a test of parsing this attribute. t/page/document.t tests the behaviour of the attribute, but not the parsing of the attribute from frontmatter into a Statocles::Document.

What say you?

Thanks!

-CXW

preaction commented 4 years ago

I say I'm starting to see why folks hate YAML 🙁. IIRC tinita has been working on making YAML::PP better, so Statocles should probably use that. But I also thought she was making YAML.pm a frontend to the (compatible) backend libraries (that work may be still underway).

I think for now swapping to YAML::PP is probably the best option. Statocles v0 is in maintenance mode until the v2 branch is ready (which uses Yancy::Backend::Static to parse the documents, which may also have this same problem), so drastic changes here aren't that valuable.

Thanks for the report!