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

Document how mentions (pills) work #1547

Merged
merged 5 commits into from Aug 29, 2018

Conversation

4 participants
@turt2live
Member

turt2live commented Aug 22, 2018

Rendered: see 'docs' status check


Implements the proposal over at #1067

Includes some specification for how matrix.to is structured, and how it is intended to be replaced.

Closes #1067

Document how mentions (pills) work
Implements the proposal over at #1067

Includes some specification for how matrix.to is structured, and how it is intended to be replaced.

@turt2live turt2live added this to In review in August 2018 r0 via automation Aug 22, 2018

@turt2live turt2live requested a review from matrix-org/spec-core-team Aug 22, 2018

this may be done by changing the background color of the mention to indicate
that it is different from a normal link.
If the current user is mentioned in a message (either by a mention as defined

This comment has been minimized.

@Half-Shot

Half-Shot Aug 22, 2018

Contributor

Should we define that client's shouldn't match on formatted_body, and only on body or is that a push rule thing?

This comment has been minimized.

@turt2live

turt2live Aug 22, 2018

Member

That's a push rule thing. This particular blob of text is intended to say that client should do something special about mentions, however.

@KitsuneRal

The comment on matrix.to is the blocking one for me; the other one is obviously not critical.

----------------
In addition to using the appropriate ``matrix.to URI`` for the mention,
clients should use the following guidelines when making mentions:

This comment has been minimized.

@KitsuneRal

KitsuneRal Aug 25, 2018

Contributor

Can it be "...when making mentions in events to be sent"? The first time I've read the text, this part looked ambiguous, whether it's about rendering the received mentions or creating mentions to be sent.

++++++++++++++++++++
.. NOTE:
This namespacing is in place pending a ``matrix://`` (or similar) URI scheme.

This comment has been minimized.

@KitsuneRal

KitsuneRal Aug 25, 2018

Contributor

To address a concern of @non-Jedi in #1067: can we add somewhere that this URI scheme does not necessarily imply a working switchboard at an actual https://matrix.to server and that clients are explicitly discouraged from using matrix.to URIs as ordinary HTTPS URLs when it comes to internal navigation between Matrix resources?

This comment has been minimized.

@Half-Shot

Half-Shot Aug 25, 2018

Contributor

+1 to this, it's very easy for client developers to miss that this should ideally provoke an action in the client rather than opening matrix.to.

This comment has been minimized.

@turt2live

turt2live Aug 25, 2018

Member

Later on the text explains how the URL is intended to work, is it not sufficient?

I'd rather not have lots of text in notes, as it no longer becomes a note.

This comment has been minimized.

@turt2live

turt2live Aug 26, 2018

Member

I added some additional text to this section to describe how it is supposed to work. The "later on in the text" part of my last comment was referring to the client-server API, which is a fair ways away from the appendices.

This comment has been minimized.

@ara4n

ara4n Aug 28, 2018

Member

i agree that we should spell out here lout and clear on the first line (not as a note) that: "matrix.to" URIs are intended to be used for namespacing, not accessing the webservice at matrix.to (other than for fallback purposes).

This comment has been minimized.

@turt2live

turt2live Aug 28, 2018

Member

@ara4n is the improved text (312799a) any better?

This comment has been minimized.

@ara4n

ara4n Aug 28, 2018

Member

i think we need to hoist it higher (e.g. line 262) and be blunter. Putting it at the end makes it seem unimportant, whereas i really think we need to address it head on as otherwise everyone will immediately assume it's a normal URL.

This comment has been minimized.

@turt2live

turt2live Aug 28, 2018

Member

So I noticed that the top level note wasn't rendering in the RST at all, and have fixed that. It also includes a fairly blunt mention that it's not backed by a webservice and to read on for details.

August 2018 r0 automation moved this from In review (just the PRs) to Reviewer approved Aug 26, 2018

@KitsuneRal

Thank you!

Examples of matrix.to URIs are:
* Room: ``https://matrix.to/#/!somewhere:domain.com``

This comment has been minimized.

@ara4n

ara4n Aug 28, 2018

Member

We should list the room alias variants before the room ID variants, and put a TODO or NOTE that the room ID ones are completely insufficient for use as URLs (unless we were to put a ?server=foo.com on them in order to identify a server which should be able to seed that ID, or unless we explicitly parse the domain out of the ID), linking perhaps to vector-im/riot-web#2925

@ara4n

lgtm, other than wanting to shout louder that matrix.to is just for namespacing here; the web service is not involved and mentions have no centralised dependency on DNS on that webapp, and wanting to acknowledge that room-id based permalinks are totally broken.

August 2018 r0 automation moved this from Reviewer approved to In review (just the PRs) Aug 28, 2018

turt2live added some commits Aug 28, 2018

Clarify what matrix.to is and mention that room IDs are not routable
Also actually render the warning saying that this scheme is temporary.

@turt2live turt2live requested review from ara4n, Half-Shot and KitsuneRal Aug 28, 2018

August 2018 r0 automation moved this from In review (just the PRs) to Reviewer approved Aug 28, 2018

@ara4n

ara4n approved these changes Aug 28, 2018

lgtm now

@Half-Shot

LGTM

@turt2live turt2live merged commit 39ef845 into matrix-org:master Aug 29, 2018

4 checks passed

ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details

August 2018 r0 automation moved this from Reviewer approved to Done (this list will be incomplete) Aug 29, 2018

@turt2live turt2live deleted the turt2live:travis/c2s/pills branch Aug 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment