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

Describe Matrix URI scheme #3168

Closed
wants to merge 2 commits into from
Closed

Conversation

turt2live
Copy link
Member

Spec for #2312

@turt2live turt2live requested a review from a team May 2, 2021 04:28
Copy link
Contributor

@ShadowJonathan ShadowJonathan left a comment

Choose a reason for hiding this comment

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

(Some comments, close them it if it's not requested from the community to comment on these PRs, I just have some nits)

Comment on lines +634 to +636
There are two major kinds of referring to a resource in Matrix: matrix.to
and `matrix:` URI. The specification currently defines both as active/valid
ways to refer to entities/resources.
Copy link
Contributor

Choose a reason for hiding this comment

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

No preference/note of deprecation of the former?

Copy link
Member Author

Choose a reason for hiding this comment

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

The MSC didn't deprecate anything


Note that this format is deliberately following [RFC 3986](https://tools.ietf.org/html/rfc3986)
to ensure maximum compatibility with existing tooling. This URI scheme is
registered with the IANA [here](https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
registered with the IANA [here](https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml).
registered with the IANA [here](https://www.iana.org/assignments/uri-schemes/prov/matrix).

Copy link
Member Author

Choose a reason for hiding this comment

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

Long term I'd expect that URL to change, so didn't want to have a broken link.

service - see below for more details.
During development of this URI format, types of `user`, `room`, and `event`
were used: these MUST NOT be produced any further, though implementations might
wish to consider handling them as `u`, `r`, and `e` respectively.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
wish to consider handling them as `u`, `r`, and `e` respectively.
wish to consider handling them as `u`, `r`, and `e` respectively.
`roomid` will be used as indicated in development.

(for exhaustiveness' sake)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure this is important, honestly. The note isn't meant to be comprehensive, just an indicator that "weird" data might be present

Comment on lines +737 to +738
* Permalink by room: `matrix:r/somewhere:example.org/e/event:example.org`
* Permalink by room ID: `matrix:roomid/somewhere:example.org/e/event:example.org?via=elsewhere.ca`
Copy link
Contributor

@ShadowJonathan ShadowJonathan May 2, 2021

Choose a reason for hiding this comment

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

Suggested change
* Permalink by room: `matrix:r/somewhere:example.org/e/event:example.org`
* Permalink by room ID: `matrix:roomid/somewhere:example.org/e/event:example.org?via=elsewhere.ca`
* Event permalink by room: `matrix:r/somewhere:example.org/e/abcdef123456`
* Event permalink by room ID (with routing hint): `matrix:roomid/somewhere:example.org/e/abcdef123456?via=elsewhere.ca`
  • "Event" permalink, for explicitness' sake
  • utilization of alphanumeric event IDs, as those are the new norm
  • "routing hint"

Copy link
Member Author

Choose a reason for hiding this comment

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

It's important that we keep the examples as similar as possible between the URIs. The v1 event IDs are clearer and appear less random.

Copy link
Member Author

Choose a reason for hiding this comment

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

(though almost a year later v1 event IDs look more confusing to people - will fix)

@turt2live
Copy link
Member Author

Community members are always welcome to comment on any PR: otherwise we would have made them private :)

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Looks plausible - someone more involved with the MSC should probably review too though

@turt2live
Copy link
Member Author

I've elected @KitsuneRal for being an author, @richvdh for having a recognisable avatar when counting comments, and @uhoreg for being responsible for registering it with IANA. Any one of the 3 will do, I think.

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

Generally LGTM, except one point (and a few nitpicks).

content/appendices.md Outdated Show resolved Hide resolved
content/appendices.md Outdated Show resolved Hide resolved
content/appendices.md Show resolved Hide resolved
content/appendices.md Outdated Show resolved Hide resolved
@richvdh richvdh removed request for uhoreg and richvdh May 11, 2021 15:48
Co-authored-by: Alexey Rusakov <Kitsune-Ral@users.sf.net>
@turt2live
Copy link
Member Author

Going to close this and make an all-new PR against the main branch. Will bring feedback over with it :)

@turt2live turt2live closed this Dec 29, 2021
@turt2live turt2live deleted the travis/spec/msc2312-uri branch December 29, 2021 18:34
turt2live added a commit that referenced this pull request Dec 30, 2021
With light review being addressed.

#3168
turt2live added a commit that referenced this pull request Dec 31, 2021
* Copy spec PR near-verbatim from past PR

With light review being addressed.

#3168

* Alter for modern day

* Add changelog

* specify that we're using the grammar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants