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.
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
MSC3015: Room state personal overrides #3015
base: old_master
Are you sure you want to change the base?
MSC3015: Room state personal overrides #3015
Changes from all commits
e0e7133
96fef8b
8065221
0de8e47
45c91fd
a226142
616dcba
6b5ccbe
e343893
d448559
c33ef1a
ad7f2fa
dd423aa
76e0522
9a7f694
910be82
abc7edf
108ffa7
2be176c
eea1ba0
87bfff2
ba19693
6147bd6
6da8d3a
fa6f59b
2e8536e
0eb7773
00ade35
5df15b6
a470fba
39c08c4
12c0a3f
21f14c2
cfb8f55
ffad669
6ff1393
f0e6037
25a6d58
381103a
979eb47
016d3f8
41733af
359b003
985f30e
50cfe4f
74a25c6
1bd1132
e9d925b
cc88c7e
f859140
415a7f4
8c1fbce
34acaed
54795fc
649b5b4
42e5c7f
3cc130e
a56f5aa
9c94f4c
5ba4368
74fa075
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming keys that way effectively imposes a new top-level namespace, which we don't have as yet. Any particular reasons against
m.room.name.override
(and so on)? You already use this form in the unstable prefix section, by the way.(I tend to agree that a marker that this is a user-specific override, not a part of the server-tracked room state, is important).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By making an new top-level namespace, I tried to separate overrides from other groups of namespaces, to not mix different things in one namespace. But if top level namespace is bad idea, I can rework this to use suffix instead of prefix...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primary problem with adding a top-level namespace here is that we use reverse-DNS notation, and can't guarantee that
override
won't end up being a legit DNS TLD. More formally, it would probably be the second occasion when we use a non-m.
, non-revDNS top-level namespace, and I'd rather follow the existing overarching convention. If you're not quite happy withoverride
as a postfix,m.override.room.*
(andorg.matrix.msc3015.override.room.*
for the unstable form - mind thatm.
can be replaced with the unstable prefix in this case) should probably be fine too.(For the record, the first non-
m.
top-level namespace wasu.
, for room tags; I don't advise employing it here sinceu.
tags are defined at the discretion of end users, not the spec.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if someone wants to override the room name with an empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this task value for
override.m.room.name
must be:like regular room state item, when room admin removes the name of room.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess it's worth putting that in the MSC text, for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would happen if user override it as empty string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Room topic is generally a message from the room admins, so I don't think it's really something that people should be overriding. (Similar for message pinning.) I can see the value in being able to add your own notes on a room and bookmark certain messages in a room, but I think those should be separate information, in addition to the topic and pinned messages, rather than overriding those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can store inside our metadata object copy of original value.
And later client app can compare it with current value for room (or user if that 1on1 room) and if it changed -- render exclamation mark.