json-ld / yaml-ld

CG specification for YAML-LD and UCR
https://json-ld.github.io/yaml-ld/spec
Other
19 stars 8 forks source link

Get rid of YAML 1.2 version declaration in spec & tests #124

Closed anatoly-scherbakov closed 8 months ago

ioggstream commented 9 months ago

Hi @anatoly-scherbakov sorry for disappearing :( Hopefully, YAML media type registration is almost done and +yaml is here https://www.iana.org/assignments/media-type-structured-suffix/media-type-structured-suffix.xhtml This means that soon we can register the application/ld+yaml media type.

I suggest to leave %YAML version declarations to avoid issues, including https://github.com/ietf-wg-httpapi/mediatypes/blob/main/draft-ietf-httpapi-yaml-mediatypes.md#expressing-booleans

Have a nice day, R.

anatoly-scherbakov commented 8 months ago

@ioggstream sorry for late reply.

I think we explicitly say that we use YAML 1.2.2, which I guess resolves that problem. Do we not?

ioggstream commented 8 months ago

But how do we tell to a parser library that it must use 1.2? It can't be implicit

TallTed commented 8 months ago

Well... We have lines 64-73 --

      "YAML": {
        title: "YAML Ain’t Markup Language (YAML™) version 1.2.2",
        href: "https://yaml.org/spec/1.2.2/",
        date: "2021-10-01",
        authors: [
          "Oren Ben-Kiki",
          "Clark Evans",
          "Ingy döt Net"
        ]
      },

-- and lines 764-770 --

    <p>
      This document is based on YAML 1.2.2,
      but YAML-LD is not tied to a specific version of YAML.
      Implementers concerned about features related to a specific YAML version
      can specify it in documents using the `%YAML` directive
      (see <a href="#int" class="sectionRef"></a>).
    </p>

-- and lines 972-979 --

    <p>
      The YAML-LD format and the media type registration are not restricted to a specific
      version of YAML,
      but implementers that want to use YAML-LD with YAML versions
      other than 1.2.2 need to be aware that the considerations and analysis provided
      here, including interoperability and security considerations, are based
      on the YAML 1.2.2 specification.
    </p>

-- and lines 1849-1861 --

          <p>
            Although outside of the scope of this specification,
            processors MAY use
            <a>YAML directives</a>, including <a>TAG directives</a>, and
            <a data-cite="YAML#document-markers">Document markers</a>,
            as appropriate for best results.
            Specifically, if the {{JsonLdOptions/extendedYAML}} API flag is `true`,
            the document SHOULD use the `%YAML` directive with
            version set to at least `1.2`.
            To improve readability and reduce document size,
            the document MAY use a `%TAG` directive appropriate for
            <a>RDF literals</a> contained within the representation.
          </p>

Today, we also have line 1879 --

      %YAML 1.2

-- and line 2529 --

              %YAML 1.2

-- but these two lines will be deleted by #125.

Is the remainder sufficient for your concern, @ioggstream?

ioggstream commented 8 months ago

The problem is not in the spec.

If you need to run Yaml code in the testlist, the header ensures that the parser processes the file according to the specified version (e.g. a parser may default to Yaml 1.x without the %YAML 1.2 header).

TallTed commented 8 months ago

@anatoly-scherbakov — From my reading of the comments above, #125 should not have been merged, as it does not solve a problem; rather, it causes one.

I do not see a problem statement from anyone above, that would lead to removing the YAML 1.2 declarations.

I do see a problem statement from @ioggstream that I think should lead to adding (replacing) the YAML 1.2 declarations.

anatoly-scherbakov commented 8 months ago

I should have written a description with a motivation for this issue. I'm sorry; I thought we did discuss it at the previous meeting. Let me try redeeming myself.

Motivation. Spec and especially the test suite use %YAML 1.2 header in YAML files. This should indicate to the parser which YAML version to use.

The spec, however, reiterates explicitly that YAML-LD is based on YAML 1.2+, as @TallTed had said in a comment above.

If the test suite runner complies with the spec, it should use YAML 1.2 by default.

Therefore, %YAML 1.2 declaration in the test suite is redundant and can be removed.

Also, the declaration can be removed from examples in the spec, to alleviate a possible feeling on the users that such a header is required.

graph LR
    yaml12("YAML ⩾ 1.2")

    subgraph test-engine-version [Test engine complies to the spec]
        compliant-test-runner("Compliant test engine") --uses--> yaml12
    end

    subgraph spec-yaml-version [Spec restricts YAML version]
        spec("YAML-LD spec") --"requires"--> yaml122("YAML ⩾ 1.2")
    end

    subgraph header-redundant [Engine does not need the version header]
        header("<code>%YAML 1.2</code> header") --is--> redundant("Redundant")
    end

    spec-yaml-version --therefore--> test-engine-version

    test-engine-version --therefore--> header-redundant --therefore--> remove-header("Remove the header!")

    comment("💬 Comment<br>© TallTed") --demonstrates--> spec-yaml-version

    click comment "https://github.com/json-ld/yaml-ld/issues/124#issuecomment-1765129903"
    click remove-header "https://github.com/json-ld/yaml-ld/pull/125"
anatoly-scherbakov commented 8 months ago

We've been discussing this on the JSON-LD CG meeting today and we seem to think that we can keep the %YAML header away.

We can add it back later if we find problems/inconsistencies.