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

input: Convert root into object, require version #420

Merged
merged 2 commits into from
Jun 3, 2022
Merged

Conversation

bdarcus
Copy link
Member

@bdarcus bdarcus commented Jun 2, 2022

Convert the root of schema into an object with metadata, and a required
version property.

Include optional title and description properties.

While I'm at it, also removing the edtf regex on a separate commit.

Close #319


Examples

Aside from this being a better, forward-looking, design, it's also more consistent with the pandoc YAML model.

The below shows this change, but in larger context of the other changes suggested in the v1.1 branch; notably with dates and titles.

An example, validated against the schema in YAML, with EDTF dates (though the regex here doesn't cover the date-time example ATM, so the validator throws an error on that property; really need a proper EDTF regex, which is not my skillset, or maybe just drop the constraint for now; see #421).

---
version: 1.1
description: An example CSL YAML 1.1 file
references:
  - type: book
    id: test1
    issued: '2020'
    title: 
      main: Title
      sub:
        - Subtitle
    author:
      - given: Jane
        family: Doe
  - type: article-journal
    id: test2
    issued: 2020-07/2020-08
    title: Simple title
    author:
      - given: John
        family: Smith
  - type: post
    id: test3
    title: “Most kids are fine!”, people say
    issued: 2022-06-02T01:57:29Z
    URL: https://twitter.com/zeynep/status/1532356192567189506

... and converted to JSON:

{
  "version": 1.1,
  "description": "An example CSL YAML 1.1 file",
  "references": [
    {
      "type": "book",
      "id": "test1",
      "issued": "2020",
      "title": {
        "main": "Title",
        "sub": [
          "Subtitle"
        ]
      },
      "author": [
        {
          "given": "Jane",
          "family": "Doe"
        }
      ]
    },
    {
      "type": "article-journal",
      "id": "test2",
      "issued": "2020-07/2020-08",
      "title": "Simple title",
      "author": [
        {
          "given": "John",
          "family": "Smith"
        }
      ]
    },
    {
      "type": "post",
      "id": "test3",
      "title": "“Most kids are fine!”, people say",
      "issued": "2022-06-02T01:57:29.000Z",
      "URL": "https://twitter.com/zeynep/status/1532356192567189506"
    }
  ]
}

... with schema-backed completion in VSCode:

Screenshot from 2022-06-01 21-09-33

Seems this ecosystem has evolved in the past two years. Once we settle on a final version, we really need to get this schema published on https://www.schemastore.org/json/, which makes editor support all the easier.

cc @jgm @bwiernik @denismaier @cormacrelf @retorquere

@bdarcus bdarcus force-pushed the json-object branch 4 times, most recently from 5b7ea26 to 0246889 Compare June 2, 2022 11:34
@bdarcus bdarcus mentioned this pull request Jun 2, 2022
4 tasks
@bdarcus bdarcus force-pushed the json-object branch 2 times, most recently from e1074de to 6bdb443 Compare June 2, 2022 12:46
Convert the root of schema into an object with metadata, and a required
'version' property.

Include optional 'title' and 'description' properties.

Also, update tests.

Close #319
@retorquere
Copy link

Regexes cannot realistically test for valid edtf dates across the range of possible dates/date ranges/date combos expressable in edtf, so it'd either be too lenient or too restrictive.

@bdarcus
Copy link
Member Author

bdarcus commented Jun 2, 2022

Regexes cannot realistically test for valid edtf dates across the range of possible dates/date ranges/date combos expressable in edtf, so it'd either be too lenient or too restrictive.

I thought you might have something to say about this; thanks!

Seems better to be too lenient, given the purpose.

@bdarcus bdarcus added the 1.1 label Jun 2, 2022
@bdarcus bdarcus added this to the CSL 1.1 milestone Jun 2, 2022
@bdarcus
Copy link
Member Author

bdarcus commented Jun 3, 2022

@retorquere - do you think I should just remove the regex "pattern" here entirely?

    "edtf-string": {
      "title": "EDTF date string",
      "description": "CSL input supports EDTF, levels 0 and 1.",
      "type": "string",
      "pattern": "^[0-9-%~X?./]{4,}$"
    }

Or do you have a suggestion for a more lenient regex, that would still be useful?

@retorquere
Copy link

retorquere commented Jun 3, 2022

I've been trying to find the place where I got it without success but I think I read at some point that EDTF dates are not regular, so regexes are problematic to parse/validate them. I know just enough about EDTF to leave validation to @inukshuk.

Personally, I'd add edtf validation as custom format attributes, and provide a reference implementation using ajv + EDTF.js. If I'm reading https://json-schema.org/draft/2020-12/json-schema-validation.html#rfc.section.7.2.3 right, processors that don't know about these custom properties would validate presence but not format of the property.

edit: the regex above would reject date sequences and significant-digit markers (S), letter-prefixed calendar year (Y), and dates with time.

@bdarcus
Copy link
Member Author

bdarcus commented Jun 3, 2022

the regex above would reject date sequences and significant-digit markers (S), letter-prefixed calendar year (Y), and dates with time.

The last is important for us. Are the others?

@retorquere
Copy link

I don't know, honestly. Does the processor do anything with date sequences? Letter-prefixed seems valid, even if I've not seen it in the wild yet.

This may not be the ideal long-term solution, but a regex isn't
well-suited to validating an edtf string.

In the future, we may want to use a dedicated 'format' property here.

See #421
@bdarcus bdarcus merged commit 85c1c83 into v1.1 Jun 3, 2022
@bdarcus bdarcus mentioned this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants