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

Unable to reply to a multi-line message using any line's short thread ID #480

Closed
kot0dama opened this issue Oct 14, 2022 · 4 comments · Fixed by #482
Closed

Unable to reply to a multi-line message using any line's short thread ID #480

kot0dama opened this issue Oct 14, 2022 · 4 comments · Fixed by #482

Comments

@kot0dama
Copy link

When using short thread IDs, if someone sends a multi-line message, matterircd sends the messages to the client with auto-incremented short IDs for each line of the message.

Then when an IRC user wants to reply to the thread, it seems they need to use the last short thread-ID only, else matterircd will fail to detect which thread it belongs to, and will send @@xxx prefix in the message, without a warning it could not find the thread ID.

Also, there is absolutely no indication that a received line is part of a multi-line message, or that every line of these message belong to a specific thread ID, hence there is no way for a user to know they should reply using the last message's thread ID.

Example from IRC client side:
matterircd_threadid_issues_hexchat_side

From Mattermost side:
matterircd_threadid_issues_mm_side

Thank you

42wim added a commit that referenced this issue Oct 14, 2022
This fixes #480 and as a bonus
also fixes #426 as this is now
just 1 big message for irc
@42wim
Copy link
Owner

42wim commented Oct 15, 2022

Try PR #482 this should fix the issue

hloeung pushed a commit to hloeung/matterircd that referenced this issue Oct 16, 2022
This fixes 42wim#480 and as a bonus
also fixes 42wim#426 as this is now
just 1 big message for irc
@kot0dama
Copy link
Author

Thanks for this, now we can clearly identify multi line messages.

Although, if I might nitpick, this change breaks 2 things:

  • message highlighting by IRC clients as only the last line gets the mention bit.
  • plugins coloring thread IDs by their number to help the user identify threads messages as the thread ID is not prefixed to every line

Maybe we need to also prefix current thread ID and suffix re: / mention items when breaking the line before sending to IRC ?

@hloeung
Copy link
Collaborator

hloeung commented Oct 17, 2022

Follow up PR #483 to @42wim's PR #482 fixes the issue with having each line have the thread ID/context.

As for each line having the mention, as it is right now, it's only the last line. Would be a little noisy if it's highlighting on each and every message of a multi-line message, IMO.

@kot0dama
Copy link
Author

Nice, thanks!
I think having mentions on only one line is OK, it's just that we had them on every line of a multi-line message before the change.

@42wim 42wim closed this as completed in #482 Nov 1, 2022
42wim added a commit that referenced this issue Nov 1, 2022
* Do not split message on newlines

This fixes #480 and as a bonus
also fixes #426 as this is now
just 1 big message for irc

See also #483

* Check code block start on multiple lines

* Ensure prefixcontext/suffixcontent shown for all lines in a multi-line message (#483)

* Ensure prefixcontext/suffixcontent shown for all lines in a multi-line message

* Make showing context for multi-line messages configurable

* Fix to handle empty lines now that we're splitting elsewhere

* Restore changes to MsgSpoofUser() and Spoof() per review

* Allow overriding max. line length

* Fix to correctly add message thread context

* Refactor and remove duplicate code per review

Co-authored-by: Haw Loeung <haw.loeung@canonical.com>
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 a pull request may close this issue.

3 participants