matrix-org / matrix-spec

The Matrix protocol specification
Apache License 2.0
171 stars 91 forks source link

Add support for pattern formats for `patternProperties` #1796

Closed zecakeh closed 2 months ago

zecakeh commented 2 months ago

The idea here is to be able to identify more easily what strings represent in the rendered spec.

If this is accepted, then we can chose most additionalProperties to patternProperties throughout the specification. We can even extend the support to the regular format key on string properties.

Here is what that changes in the rendered specification, for m.receipt:

Before:

Capture d’écran du 2024-04-23 15-08-50

After:

image

Preview: https://pr1796--matrix-spec-previews.netlify.app

TheArcaneBrony commented 2 months ago

Personally not sure if this change makes sense, given that column is meant to show the data type. I wonder if something like {string user_id: Receipt data} would work better?

zecakeh commented 2 months ago

It looks really nice but I feel a bit uneasy that we end up with same regexes spelled out two times, in different files. And I might be missing something but you don't seem to actually use pattern defined in custom-formats.yaml?

As said in the docs of the file, the regex is only here as a reference to help for creating new patternProperties. It can safely be removed if it's bothering you.

richvdh commented 2 months ago

Personally not sure if this change makes sense, given that column is meant to show the data type. I wonder if something like {string user_id: Receipt data} would work better?

I realised we didn't reply to this (pro-tip: comments on the diff are more likely to get noticed than comments on the PR itself). Given this is JSON, the only thing that can be used as an object key is a string. I don't think there is any need to specify string for every such mapping.