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

Clarify that a region requires a "start" property. #462

Closed
ghost opened this issue Jun 8, 2020 · 2 comments
Closed

Clarify that a region requires a "start" property. #462

ghost opened this issue Jun 8, 2020 · 2 comments
Assignees
Labels
2.1.0-erratum design-approved The TC approved the design and I can write the change draft impact-non-breaking-change ready

Comments

@ghost
Copy link

ghost commented Jun 8, 2020

The spec says:

  • Every region is one (or more) of a text region with line/column properties, a text region with character offset/length properties, or a binary region (see §3.30.1).
  • A text region specified by line/column properties requires a startLine property (§3.30.5).
  • A text region specified by character offset/length properties requires a charOffset property (§3.30.9).
  • A binary region requires a byteOffset property (§3.30.11).

From all this we can conclude that at least one of startLine, charOffset, or byteOffset is required. But the spec doesn't come right out and say that, and the schema doesn't require it.

Clarify this point in the spec text, and add the constraint to the schema's definition of the region object:

"region": {
  ...
  "anyOf": [
    { "required": [ "startLine" ] },
    { "required": [ "charOffset" ] },
    { "required": [ "byteOffset" ] }
  ]

@michaelcfanning and @harleenkohli FYI (Harleen and I ran into this point today).

@sthagen
Copy link
Contributor

sthagen commented Dec 15, 2023

Propose to close as already resolved with the errata01 of the OS standard.
@dmk42 Do I got that right?

References:

@sthagen sthagen assigned dmk42 and unassigned sthagen Dec 15, 2023
@dmk42
Copy link
Contributor

dmk42 commented Dec 19, 2023

Now that the Errata document has been published, this has been fixed in the official version of the standard.

@dmk42 dmk42 closed this as completed Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.1.0-erratum design-approved The TC approved the design and I can write the change draft impact-non-breaking-change ready
Projects
None yet
Development

No branches or pull requests

4 participants