Opened 3 years ago

Closed 20 months ago

#4488 closed change (fixed)

Support JSON page front matter

Reported by: juliandoucette Assignee: rhowell
Priority: P4 Milestone:
Module: Sitescripts Keywords:
Cc: kvas, saroyanm, fhd, sebastian Blocked By: #4491
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29516687

Description (last modified by kvas)

Background

Our CMS page front matter format supports key=value syntax.

Although this is effective, it is inconvenient because this format does not support arrays or objects.

What to change

Implement support for JSON format front matter (without breaking backwards compatibility).

Examples

These should work with/without HTML comments:

Original / Backwards compatible

<!--
title = Awesome page
description = Awesome description
image_url = /img/awesome.jpg
image_type = image/jpg
image_width = 1024
image_height = 768
image_alt = Awesome image
-->

With this front matter the variables title, description, image_url, image_type, etc. become available to use in the evaluated expressions inside of the page.

JSON

<!-- {
  "title": "Awesome page",
  "description": "Awesome description",
  "image": {
    "url": "/img/awesome.jpg",
    "type": "image/jpg",
    "width": "1024",
    "height": "768",
    "alt": "Awesome image"
  },
  "tags": ["awesome", "news"]
} -->

With this front matter the variables title, description, image['url'], image['type'], tags[0], etc. become available to use in the evaluated expressions inside of the page.

Change History (31)

comment:1 Changed 3 years ago by juliandoucette

  • Summary changed from Support JSON front matter to Support JSON front matter on CMS pages

comment:2 Changed 3 years ago by juliandoucette

  • Summary changed from Support JSON front matter on CMS pages to Support JSON front matter in CMS pages

comment:3 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:4 Changed 3 years ago by trev

By surrounding front matter JSON in a <script type="application/json"> tag

As if JSON alone wasn't enough syntactic overhead already :-)

If you are worried about editors I would suggest going with HTML comments: <!-- ... -->. Positive side-effect: this syntax doesn't suggest that the block will be part of the output.

comment:5 Changed 3 years ago by juliandoucette

If you are worried about editors I would suggest going with HTML comments: <!-- ... -->. Positive side-effect: this syntax doesn't suggest that the block will be part of the output.

Good idea.

comment:6 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:7 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:8 Changed 3 years ago by trev

  • Description modified (diff)

comment:9 Changed 3 years ago by sebastian

  • Cc sebastian added

FWIW, I consider supporting JSON for the values here, and surrounding the block by HTML comments, two separate issues. Can we please split this issue up?

comment:10 follow-up: Changed 3 years ago by juliandoucette

FWIW, I consider supporting JSON for the values here, and surrounding the block by HTML comments, two separate issues. Can we please split this issue up?

Yes.

comment:11 in reply to: ↑ 10 Changed 3 years ago by juliandoucette

FWIW, I consider supporting JSON for the values here, and surrounding the block by HTML comments, two separate issues. Can we please split this issue up?

Done.

https://issues.adblockplus.org/ticket/4491#ticket

comment:12 Changed 3 years ago by juliandoucette

  • Description modified (diff)
  • Summary changed from Support JSON front matter in CMS pages to Support JSON page front matter

comment:13 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:14 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:15 Changed 3 years ago by sebastian

Can you add some examples, how the new syntax would look like (also one considering the backwards compatibility case)?

comment:16 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:17 follow-up: Changed 3 years ago by juliandoucette

Can you add some examples, how the new syntax would look like (also one considering the backwards compatibility case)?

Done.

NOTE: As I mentioned in the last frontend developer meeting, I think this should be a low priority compared to other issues (EG: separating the front matter from the content) because we rarely need page specific arrays/objects and it is relatively easy to work around. I imagine this becoming a priority if we decide to go forward with building a WYSIWYG tool. (I got this idea because I prototyped an offline WYSIWYG HTML editor using NodeJS + Electron in my spare time a few weekends ago.)

Last edited 3 years ago by juliandoucette (previous) (diff)

comment:18 Changed 3 years ago by juliandoucette

  • Description modified (diff)

comment:19 in reply to: ↑ 17 ; follow-up: Changed 3 years ago by sebastian

I actually thought we are just going to use JSON for the values (e.g.title = "Awesome page"). That way it would easier to migrate from the previous format, and the implementation could share more code (basically we would just interpret the value as a string if it doesn't parse as JSON). I wouldn't insist though, but I guess it's good that I asked for an example, because it wasn't completely clear from the description. However, if we use the syntax that you suggest, it might make sense to merge #4491 back into this issue, and use the presence/absence of <!-- to indicate which format is used.

Replying to juliandoucette:

NOTE: As I mentioned in the last frontend developer meeting, I think this should be a low priority compared to other issues (EG: separating the front matter from the content)

Wait, do you imply that you don't want to have that data structure in the pages/* files in the first place? Otherwise, I'm not sure what you mean there. But if I understand correctly, I guess there is not much of a point to change the format here, if we are going to move that data structure somewhere else anyway.

Last edited 3 years ago by sebastian (previous) (diff)

comment:20 in reply to: ↑ 19 ; follow-up: Changed 3 years ago by juliandoucette

However, if we use the syntax that you suggest, it might make sense to merge #4491 back into this issue, and use the presence/absence of <!-- to indicate which format is used.

I disagree. The background reasoning in #4491 applies to the old format too.

Wait, do you imply that you don't want to have that data structure in the pages/* files in the first place? Otherwise, I'm not sure what you mean there. But if I understand correctly, I guess there is not much of a point to change the format here, if we are going to move that data structure somewhere else anyway.

I didn't intend to imply that.

  • It's obvious that we should support a better front matter format (EG: JSON, YAML, INI)
  • But we don't need to yet because we can use long_names_with_underscores_and_numbers and Jinja2 variables in most cases
Last edited 3 years ago by juliandoucette (previous) (diff)

comment:21 in reply to: ↑ 20 Changed 3 years ago by sebastian

Replying to juliandoucette:

I disagree. The background reasoning in #4491 applies to the old format too.

Sure, but we need a way to distinguish both formats, and I suppose there is no reason to use HTML comments, but not the JSON syntax, once both is supported. But never mind, it seems there are also other ways to detect JSON here.

I didn't intend to imply that.

So, what do you mean by "separating the front matter from the content"?

comment:22 Changed 3 years ago by juliandoucette

Sure, but we need a way to distinguish both formats

JSON starts with "{"

and I suppose there is no reason to use HTML comments, but not the JSON syntax, once both is supported.

We would use HTML comments to separate the front matter from the content in the code so that they are ignored by editor formatting, linters, and HTML/Markdown previews (EG: Packages -> Markdown Preview in Atom).

So, what do you mean by "separating the front matter from the content"?

Not separated

title=Awesome page
<h1>Awesome page

Separated

<!--
title=Awesome page
-->
<h1>Awesome page</h1>
Last edited 3 years ago by juliandoucette (previous) (diff)

comment:23 Changed 3 years ago by sebastian

  • Priority changed from Unknown to P4
  • Ready set

comment:24 Changed 2 years ago by rhowell

  • Owner set to rhowell

comment:25 Changed 2 years ago by rhowell

  • Description modified (diff)

comment:26 Changed 2 years ago by jsonesen

  • Blocked By 4491 added

comment:27 Changed 2 years ago by kvas

  • Description modified (diff)

Based on an IRC conversation with Julian, I've updated the issue to make it clear that not interpretation should be performed on JSON metadata other than JSON decoding.

comment:28 Changed 2 years ago by fhd

  • Cc trev removed

comment:29 Changed 21 months ago by sebastian

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:30 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 4488 - Add support for JSON page front matter

comment:31 Changed 20 months ago by rhowell

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.