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

Specify "if", "then", and "else" #375

Merged
merged 2 commits into from
Sep 10, 2017
Merged

Conversation

handrews
Copy link
Contributor

This addresses #180.

I have intentionally allowed for any combination of these
keywords to be present or absent. While having a "then" or "else"
without "if" is pointless and/or nonsensical, so are many other
possible JSON Schema keywords.

@handrews
Copy link
Contributor Author

handrews commented Sep 4, 2017

Paging @epoberezkin -- does this spec match your implementation?

@epoberezkin
Copy link
Member

epoberezkin commented Sep 5, 2017

@handrews I implemented it somewhat differently in terms of what is required.

In ajv-keywords then is required when if or else is present, it can be expressed as:

"dependencies": {
  "if": ["then"],
  "else": ["then"]
}

Also I considered if to have a default value of true, meaning that when if is absent, then the instance should be valid against then and invalid against else (if present).

The reason for that is that it was how switch was specified - then was allowed without if and it was required that the instance is valid against then in this case.

Maybe it's better to say that if either then or else is present (or both), if MUST be present:

"dependencies": {
  "then": ["if"],
  "else": ["if"]
}

That would make the paragraphs about ignoring then/else unnecessary. I think it's better to make the schema invalid, then ignore some parts of it in this case...

@handrews
Copy link
Contributor Author

handrews commented Sep 5, 2017

@epoberezkin that all does make sense.

My rationale for not requiring any of them to be present based on the others is mostly because of re-use patterns. I can see someone having a fairly generic if condition and a straightforward else, and then combining that with different then clauses. Admittedly, I don't have a compelling use case for this, but JSON Schema in general allows weirdly incomplete and/or useless schema keyword combinations so it seemed like the most consistent approach.

@epoberezkin
Copy link
Member

@handrews that's ok as is i think, I simply wrote how it is different.

This addresses json-schema-org#180.

I have intentionally allowed for any combination of these
keywords to be present or absent.  While having a "then" or "else"
without "if" is pointless and/or nonsensical, so are many other
possible JSON Schema keywords.
@handrews
Copy link
Contributor Author

handrews commented Sep 7, 2017

I just rebased the original commit on top of the section reorganization @dlax did, but it is otherwise unchanged. I also added a subsequent commit rewording the section title to be consistent with the one about allOf/anyOf/oneOf/not, in case anyone wants to see that. But it does not change anything significant about the PR.

@handrews handrews merged commit d9ee9e9 into json-schema-org:master Sep 10, 2017
@handrews handrews deleted the if branch September 10, 2017 23:14
@silkentrance
Copy link

silkentrance commented Sep 22, 2017

@handrews This is imperative and and does not work for schemas that well. See XML Schema for example. There was a better approach using "inverse logic" but I am unable to find it right now.

Regardless of that, the condition schema for if should be made more clear. The existing examples do not include an express form of a condition and just include references to a definition of a provable version of such a condition.

@handrews
Copy link
Contributor Author

@silkentrance Please take a look at issue #64 followed by #180 for an extremely extensive discussion of imperative vs declarative approaches, with more than 100 comments between the two issues. (Note that while I filed #64, I did not write it- I imported it from the old proposal wiki).

Could you elaborate on what you want with the if schema? It is intentionally unrestrained: validation provides a true/false outcome, which is all that is needed to decide between then and else. I'm not sure what you mean by "an express form of a condition".

As for its suitability in schemas, @epoberezkin implemented it back in December and it has gotten a fair amount of use without any problems. If you want me to look at XML Schema I'm going to need a specific link- there's way too much to slog through otherwise. I avoid XML like the plague.

@silkentrance
Copy link

@handrews Yeah, #64 and #180. I've already read a lot of the stuff that is in there...

Express form of a condition basically means to provide working examples in the specification. ATM and IMO the specification lacks proper examples. Compare for example the W3C's XML Schema specification and other specifications.

As for "slog", see the above two paragaphs 😀

@handrews
Copy link
Contributor Author

PRs welcome for examples. Or maybe @epoberezkin can help as the primary advocate for this change? I'm not trying to be a jerk but I've more than got my hands full with hyper-schema this round and no one ever likes my examples anyway. It's much better if someone else writes them.

@handrews
Copy link
Contributor Author

@silkentrance in general the validation spec has very few examples. I haven't had all that much to do with that spec, and I like it the way it is b/c I can quickly read the theory which is what I care about (this is why my examples are nearly always horrible).

In general, the idea is for there to be a lot more of a tutorial on the web site, rather than cluttering the spec with them: https://github.com/json-schema-org/json-schema-org.github.io

It might be worth an issue either here (if you want to argue for spec examples) or in the web site repo to help sort this out. Most people aren't looking at closed PRs.

I'm mostly just nudging the validation spec along with non-controversial things for this draft. My focus is really hyper-schema, and I'm hoping awwright or one of the other contributors (or someone new!) will have time to move validation forward more in the next draft after this one. I admit I'm sucking up most available time from others with hyper-schema reviews right now.

@silkentrance
Copy link

@handrews I am not pointing fingers at anybody, you are all doing a great job here! As for the PRs, I am looking into that and when I find the time, I will provide you with such.

@handrews
Copy link
Contributor Author

handrews commented Oct 1, 2017

@silkentrance thanks! My "I haven't had all that much to do with..." comment wasn't trying to direct blame, just trying to explain why I'm not jumping on this to the same degree that I am with Hyper-Schema concerns right now.

BTW, figuring out how to put in enough examples without totally breaking up the flow of the spec is something I'm wrestling with in Hyper-Schema as well.

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.

4 participants