Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Fix issue #519 #587

Closed
wants to merge 1 commit into from
Closed

Fix issue #519 #587

wants to merge 1 commit into from

Conversation

tjanc
Copy link
Contributor

@tjanc tjanc commented Aug 13, 2018

Context

  1. A fixedType Array element should be treated as the type of lists of any of the elements in its content.
  2. An Enum element should be treated as the type of any of the elements in its enumerations attribute.

One could argue both any of combinators are expected to behave similarly.

Current behaviour

In (1) the JSON value rendering precedence is content - sample - default.
In (2) the JSON value rendering precedence is content - sample - default - enumerations.

This is unified only syntactically. In (1) what seems to be content actually is a fixedType'd Array element's way to define its enumerations.

Fix

Issue #519 comes with this suggestion:
In (1) the JSON value rendering precedence SHOULD BE sample - default - content.

This PR does just this, it changes the precedence of value source for fixedType Array elements in the JSON value renderer.

@pksunkara pksunkara added this to the 4.0.0 milestone Aug 21, 2018
Copy link
Contributor

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is not about the fixed-type. It's about the sample described in array taking precedence. The #519 issue should produce the same expected JSON value even with no fixed-type flag there.

@pksunkara pksunkara modified the milestones: 4.0.0, 4.1.0 Aug 22, 2018
@kylef
Copy link
Member

kylef commented May 2, 2019

Discussed with @tjanc, we will close this PR as we should find a way to solve this more generically instead of a work-around on fixed-type specifically. Closing but leaving branch for future reference.

@kylef kylef closed this May 2, 2019
@kylef
Copy link
Member

kylef commented May 2, 2019

We'll likely have to change MSON / API Elements to be able to solve this problem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants