improve display of event subtypes #1283
Conversation
| @@ -736,7 +736,7 @@ following error codes are used in addition to those already specified: | |||
| - `m.mismatched_commitment`: The hash commitment did not match. | |||
| - `m.mismatched_sas`: The SAS did not match. | |||
|
|
|||
| {{% event event="m.key.verification.start$m.sas.v1" %}} | |||
| {{% event event="m.key.verification.start$m.sas.v1" title="`m.key.verification.start` with `method: m.sas.v1`" %}} | |||
There was a problem hiding this comment.
Any reason not to put this title in the definition itself? jsonschema defines a title attribute which seems like it would be ideal (https://json-schema.org/draft/2020-12/json-schema-validation.html#name-title-and-description)
There was a problem hiding this comment.
mainly because some event definitions already use the title attribute (e.g. https://github.com/matrix-org/matrix-spec/blob/main/data/event-schemas/schema/m.room.power_levels.yaml) and set it to something other than what the current title is in the spec.
There was a problem hiding this comment.
The example you give looks like it should be the description rather than the title.
Given we know that none of the titles are rendered, maybe we just rip out the existing ones with no loss of value?
Sorry to make the job (even) bigger, but I feel like this is going to be a maintenance headache we should just solve rather than hack around for now
There was a problem hiding this comment.
I tried doing this, and ran into a snag. All the events inherit (using allOf, usually indirectly) from core-event-schema/event.yaml, so I need to remove all the title properties from everything in the chain. However, removing the title property from event.yaml turns things like
becomes
The only way I can think of to actually get this to work the way we want it to is to modify all the event yaml files to have a title property set to the event type (e.g. title: m.room.message in m.room.message.yaml), which seem ... suboptimal.
There was a problem hiding this comment.
ugh what a pain in the butt.
It feels like really the problem here is that things like this shouldn't be referring to event.yaml anyway, and should use a more specific type (with an appropriate title).
(see also my complaints at #897)
... however, maybe fixing that right now is a bit out of scope and we should just stick with your original suggestion.
improvements to
m.<event type>$<subtype>eventsm.room.messageonly shows one example, rather than a whole bunchfixes #815
Preview: https://pr1283--matrix-spec-previews.netlify.app