Do not use title for objects containing only additionalProperties or patternProperties
#1801
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…patternProperties Previously, titles would appear that do not link to a subchema definition. It would also mean that named subschemas would appear without being clearly referenced. Now, the type clearly shows the nesting of objects and subschema definitions should be clearly referenced. Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
richvdh
reviewed
Apr 24, 2024
Merged
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
richvdh
reviewed
Apr 30, 2024
| */}} | ||
| {{ define "partials/object-type-or-title" }} | ||
| {{ $type := "object" }} | ||
| {{ if .title }} | ||
| {{ if and .title (or .properties (not (or .additionalProperties .patternProperties))) }} |
There was a problem hiding this comment.
we should mention .properties in the comment for this partial.
Comment on lines
161
to
170
| {{ if and .title (or .properties (not (or .additionalProperties .patternProperties))) }} | ||
| {{/* | ||
| If the property has a `title`, use that rather than `type`. | ||
| This means we can write things like `EventFilter` rather than `object`. | ||
| We only want to use the title in two cases: | ||
|
|
||
| * The object is rendered as a separate table that will use the same | ||
| title, which means that the object must have `properties`. | ||
| * The object doesn't define any properties, because showing the title | ||
| (like `EventFilter`) is better than showing `object`. | ||
| */}} | ||
| {{ $type = .title | htmlEscape }} |
There was a problem hiding this comment.
I must say this logic makes my head explode, though I think it's probably right.
What if we did something like this instead:
{{ $title = or .title "object" }}
{{ if .anchor }}
{{ $title = printf "<a href=\"#%s\">%s</a>" (htmlEscape .anchor) $type }}
{{ end }}
{{ if .properties }}
{{/*
The object has its own (regular) properties, so we will make a separate table for it.
Refer to it using its title, if it has one.
*/}}
{{ $type = $title }}
{{ else if reflect.IsMap .additionalProperties }}
// as before
{{ else if reflect.IsMap .patternProperties }}
// as before
{{ else }}
{{/*
No properties, so there won't be a separate table. We use the title anyway, because showing the title
(like `EventFilter`) is better than showing `object`.
*/}}
{{ $type = $title }}
{{ end }}
There was a problem hiding this comment.
Yes, that's probably easier to read. I'll change it.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, titles would appear that do not link to a subschema definition. It would also mean that named subschemas would appear without being clearly referenced.
Now, the type clearly shows the nesting of objects and subschema definitions should be clearly referenced.
Examples:
Before:
After:
Before:
After:
Before:
After:
Alternatively, we could just remove the
titlefrom the objects manually, allowing us to show a title when we really want to.The rendered types can be improved later by using formats as introduced in #1796.
Preview: https://pr1801--matrix-spec-previews.netlify.app