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

MSC3226: Per-room spell check #3226

Merged
merged 0 commits into from
Aug 30, 2021
Merged

MSC3226: Per-room spell check #3226

merged 0 commits into from
Aug 30, 2021

Conversation

afranke
Copy link
Contributor

@afranke afranke commented May 31, 2021

Rendered

Signed-off-by: Alexandre Franke afranke@gnome.org

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Congratulations on your first MSC :)

We'll need sign-off so we can merge the MSC down the line: the easiest way to do this is to add a comment or edit the description to include the following template:

Signed-off-by: Your Name <email@example.org>

proposals/3225-per-room-spellcheck-language.md Outdated Show resolved Hide resolved
@turt2live turt2live changed the title Add per room spell check language MSC MSC3226: Per-room spell check May 31, 2021
@turt2live turt2live added client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels May 31, 2021
@afranke
Copy link
Contributor Author

afranke commented May 31, 2021

Congratulations on your first MSC :)

We'll need sign-off so we can merge the MSC down the line: the easiest way to do this is to add a comment or edit the description to include the following template:

Signed-off-by: Your Name <email@example.org>

✔️ Done.

Clients offering spell checking should default their dictionary to the system language. They should offer
a way to change it and when the user sets a specific language, then they should store the language code
as `m.input_language` account data for the room where it was set. Language codes must be
[RFC3066](https://datatracker.ietf.org/doc/html/rfc3066) compliant.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to say which field to store the language code in

{
  "type": "m.input_language",
  "content": {
    "???": "fi-FI"
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha, right. We currently use:

{
  "type": "org.gnome.fractal.language",
  "content": {
    "input_language": "fr_FR"
  }
}

in Fractal. Would it make sense to have m.input_language as the type and input_language as the field name?

Copy link
Member

@tulir tulir May 31, 2021

Choose a reason for hiding this comment

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

The field name isn't too important so it's probably easiest to just go with what you're already using.

If you want to bikeshed it more, here's what the spec currently has:

  • m.room.name -> name
  • m.room.topic -> topic
  • m.room.avatar -> url
  • m.room.join_rules -> join_rule
  • m.room.history_visibility -> history_visibility
  • m.room.guest_access -> guest_access
  • m.room.canonical_alias -> alias
  • m.room.pinned_events -> pinned
  • m.tag -> tags
  • m.ignored_user_list -> ignored_users
  • m.fully_read -> event_id

Comment on lines 25 to 30
The language could be defined as a room property: a room administrator could set it once and then every
member would get it for free. However, not everyone in a given room necessarily talks in the same language
(or even language variant, e.g. en_US vs en_GB) and there would need to be a way for users to override the
room property. This MSC could be used as a mechanism to do said override, if such a room property were ever
defined. Adding a room property requires more work and thought, and can be done in another MSC. It is thus
considered out of scope for this one.
Copy link
Member

Choose a reason for hiding this comment

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

Does it require that much more work and thought to say that m.input_language can be in either room state or room account data, with the room account data taking priority?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest I don’t know how room properties work that well, and I suspected it might impact room versioning. More work meant at the minimum more investigative work to see what the real impact is.

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't impact room versioning, only things that change the federation protocol (event authorization rules, state resolution, etc) need a new room version. It works pretty much the same way as account data: invent an event type and put arbitrary JSON inside.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, sounds good! I’ll update the proposal to add the room property. Thank you.


Clients offering spell checking should default their dictionary to the system language. They should offer
a way to change it and when the user sets a specific language, then they should store the language code
as `m.input_language` account data for the room where it was set. Language codes must be
Copy link

Choose a reason for hiding this comment

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

I've seen chats with multiple common languages. Would it make sense to make this an ordered list of languages based on"preference"? I would also happily add a caveat that many clients will just use the first one.

But this could be helpful for using the sum of the dictionaries or similar for spell check.

@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
@afranke afranke merged commit 70192e8 into matrix-org:old_master Aug 30, 2021
@SimonBrandner
Copy link
Contributor

@afranke, this seems to have been merged without FCP?

@afranke afranke deleted the master branch August 30, 2021 13:07
@afranke
Copy link
Contributor Author

afranke commented Aug 30, 2021

I’m so sorry, I messed up trying to adjust my fork to the renamed main branch. I’ll revert right now.

@SimonBrandner
Copy link
Contributor

I’m so sorry, I messed up trying to adjust my fork to the renamed main branch. I’ll revert right now.

No worries, just confused a couple of people :)

@turt2live turt2live added obsolete A proposal which has been overtaken by other proposals and removed proposal-in-review labels Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. obsolete A proposal which has been overtaken by other proposals proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants