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

Add support for custom management room messages #139

Closed
wants to merge 4 commits into from
Closed

Add support for custom management room messages #139

wants to merge 4 commits into from

Conversation

justinbot
Copy link
Contributor

@justinbot justinbot commented Oct 19, 2021

Adds configuration to support custom welcome messages in the management room.

Implementation is largely the same as mautrix/telegram#674

Copy link
Member

@sumnerevans sumnerevans left a comment

Choose a reason for hiding this comment

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

Seems fine to me. @tulir?

Copy link
Member

@tulir tulir left a comment

Choose a reason for hiding this comment

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

Overall this seems like it would fit better in mautrix-python's send_welcome_message, since it doesn't have many bridge-specific parts

@@ -65,6 +65,9 @@ def do_update(self, helper: ConfigUpdateHelper) -> None:
copy("bridge.contact_list_names")
copy("bridge.displayname_preference")

copy_dict("bridge.management_room_text")
Copy link
Member

Choose a reason for hiding this comment

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

The different messages should be copied individually because custom fields/messages aren't needed

# Optional extra text sent when joining a management room.
# additional_help:
# plain: "This would be some additional text in case you need it."
# html: "This would be some additional <b>text</b> in case you need it."
Copy link
Member

Choose a reason for hiding this comment

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

Separate plaintext and HTML in the options doesn't seem particularly useful, it should just have a single string value for the markdown and render it at runtime (html = render(markdown) from mautrix.util.markdown). Alternatively it could have just the HTML field and un-render it (markdown = MatrixParser(html).text from mautrix.util.formatter)

Comment on lines +111 to +115
combined_plain = ""
combined_html = ""
for plain, html in welcome_messages:
combined_plain += plain + "\n"
combined_html += html + "<br/>"
Copy link
Member

Choose a reason for hiding this comment

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

"\n".join(...) would probably be nicer, I don't think it needs a trailing newline

@justinbot
Copy link
Contributor Author

@tulir Addressed your feedback and spun out into mautrix/python#58

Once that is reviewed I can close this and mautrix/telegram#674 and replace them with PRs adding only the new configuration.

@justinbot
Copy link
Contributor Author

Superseded by #146

@justinbot justinbot closed this Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants