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

Include limited characters from Telegram replies in relayed IRC message #51

Closed
bexelbie opened this issue Apr 30, 2018 · 14 comments · Fixed by #391
Closed

Include limited characters from Telegram replies in relayed IRC message #51

bexelbie opened this issue Apr 30, 2018 · 14 comments · Fixed by #391
Labels
improvement Improves on something that already exists Telegram Issues relating to Telegram bridge
Milestone

Comments

@bexelbie
Copy link

The teleboto bot in #fedora-zh does to interesting things. Notice this exchange from the IRC side:

[alick9188] @bexelbie You're living in China?
[bexelbie] 「Re alick9188: @bexelbi...」Nope. I’ll be in Beijing for LC3 though.

First, the name from telegram is in square brackets, making it more obvious it is passed through.

Second, it quotes part of the replied to message to give greater context.

WDYT?

@jwflory jwflory added improvement Improves on something that already exists help wanted Anyone is welcome to help us with this! priority:med labels Apr 30, 2018
@jwflory jwflory added this to the v1.2 milestone Apr 30, 2018
@jwflory
Copy link
Member

jwflory commented Apr 30, 2018

I see this issue as a UI/UX enhancement for two things:

  1. Place Telegram usernames between square brackets (which I forgot, we actually can do already – there are config values for username prefix/suffix)
  2. Quote a number of characters for messages from a replied message on Telegram and relay it over it IRC

I will update this accordingly.

Implementation ideas

Quoting messages should be a configurable options. If set to true, there should also be a configuration value for the number of characters for quoting messages. Assume a default of maybe 15 characters.

When configured accordingly, the messages appear like this:

<alick9188> @bexelbie You're living in China?
<bexelbie> 「Re alick9188: @bexelbi...」Nope. I’ll be in Beijing for LC3 though.

@robbyoconnor @repkam09 @thenaterhood @xforever1313 Do any of you have thoughts or opinions on this?

@jwflory jwflory changed the title Update IRC message for Telegram Replies Include limited characters from Telegram replies in relayed IRC message Apr 30, 2018
@jwflory jwflory added the good first issue Good for newcomers label Apr 30, 2018
@xforever1313
Copy link
Member

...I swear we had a way in teleirc to determine if a message from telegram was a reply or not... Did I dream that 0_o? That would be step one.

Step two would be getting the message that was replied to. Its been a while since I looked at the Telegram Bot API... but I would assume it would include either the replied message as a string, a message object, or an ID on a message object. If its a string or message object, that's not too bad to grab, but an ID could get tricky as we'll have to make a second request to get the string of the message of that ID (or heaven forbid do caching).

The rest is straightforward, pre-pending the IRC message with the characters from the config, and making a substring of the message that was replied to.

Another thing to consider, we need to set an upper limit of the number of characters to quote. An IRC message can go up to 510 characters (512 if you count \r\n). Subtract 7 for "PRIVMSG", 50 for a worst-case Channel name, 3 for special IRC characters, 32 characters for the worst-case Telegram user name length, leaves 418 characters for an IRC message (for Chaskis, I actually round down to 400 for the maximum number of characters a message can have to be safe). 400 characters is pretty significant, so we can probably be generous with the upper limit of the number of characters to quote (like 50-100). A number of 0 could disable the feature. A negative number should raise a configuration error.

@robbyoconnor
Copy link
Contributor

I have no opinions

@jwflory jwflory modified the milestones: v1.2, v1.2.1 Oct 14, 2018
@jwflory jwflory modified the milestones: v1.2.2, v1.3 Nov 24, 2018
@jwflory jwflory modified the milestones: v1.3, v1.4 Feb 2, 2019
@jwflory jwflory added this to Backlog in TeleIRC development Feb 2, 2019
@jwflory jwflory moved this from Backlog to Next sprint in TeleIRC development Feb 9, 2019
@Tjzabel Tjzabel self-assigned this Mar 2, 2019
@jwflory
Copy link
Member

jwflory commented Mar 2, 2019

Discussed in 2019-03-02 developer meeting.


This feature is accepted for the v1.4 release. @Tjzabel will work on this feature for this sprint. His next action item is to share a short write-up of how to approach this one before the next developer meeting on March 9th.

@jwflory jwflory added priority:crit needs info Extra attention or information needed and removed help wanted Anyone is welcome to help us with this! priority:med good first issue Good for newcomers labels Mar 2, 2019
@jwflory jwflory moved this from Next sprint to Current sprint in TeleIRC development Mar 3, 2019
@jwflory
Copy link
Member

jwflory commented Mar 30, 2019

Discussed in 2019-03-30 Teleirc developer meeting.


Since @Tjzabel is currently working on #130 this week, no further updates expected on this ticket until Saturday, April 6th.

@jwflory jwflory moved this from Current sprint to Next sprint in TeleIRC development Apr 6, 2019
@jwflory
Copy link
Member

jwflory commented Apr 6, 2019

Discussed during 2019-04-06 developer meeting.


During the meeting, we took a realistic evaluation of work remaining for this sprint and what will make the next release. We changed our original plans of releasing v1.4 at the end of this month, and instead we're going for a smaller release, v1.3.1, with some of the other tasks.

This ticket will not be completed in the v1.3.1 sprint. It will remain on the v1.4 release milestone. Work on this release may not begin until August 2019.

@jwflory jwflory added priority:med and removed needs info Extra attention or information needed priority:high labels Apr 6, 2019
@Tjzabel
Copy link
Member

Tjzabel commented Apr 21, 2019

Update

Work has gone underway! Here's what's been done so far:

  • Listener to reply events has been added
  • TgReplyHandler.js file has been created, and stubbed.

It is unknown when I will get around to actually completing this issue, but at least a head start has been made. I'm projecting an early-summer deadline.

@Tjzabel
Copy link
Member

Tjzabel commented Apr 21, 2019

Also, as an update to @xforever1313's comment, a reply message is indeed a Message object in itself, so a simple check just needs to be made to see if the reply_to_message field exists within the Message object.

If it does, then we go into that message object, grab say, the first 20 characters, and prepend it to the main message. We should make the reply message length a config variable. After that, it's pretty straight forward!

@jwflory
Copy link
Member

jwflory commented Apr 21, 2019

@Tjzabel Super, thanks for digging into this one. I saw your 51-TG-replies branch. We'll keep this on the horizon for v1.4. 👍

@Tjzabel Tjzabel moved this from Next sprint to Current sprint in TeleIRC development Apr 21, 2019
@Tjzabel Tjzabel moved this from Current sprint to In progress in TeleIRC development Apr 21, 2019
@Tjzabel Tjzabel self-assigned this May 26, 2019
@Tjzabel Tjzabel removed this from the v1.4 milestone Sep 14, 2019
@Tjzabel
Copy link
Member

Tjzabel commented Sep 14, 2019

Removing from 1.4 milestone due to re-scoping.

@Tjzabel Tjzabel moved this from In progress to Current sprint in TeleIRC development Sep 14, 2019
@Tjzabel Tjzabel moved this from Current sprint to Backlog in TeleIRC development Sep 15, 2019
@Tjzabel Tjzabel removed this from Backlog in TeleIRC development Sep 15, 2019
@Tjzabel Tjzabel removed their assignment Sep 15, 2019
@jwflory jwflory added this to the v2.x.x milestone Feb 16, 2020
@jwflory jwflory added this to Backlog in TeleIRC development via automation Feb 27, 2020
@Tjzabel Tjzabel added the Telegram Issues relating to Telegram bridge label Mar 29, 2020
@brettgilio
Copy link

What is the status on this?

@jwflory
Copy link
Member

jwflory commented Nov 17, 2020

Hi @brettgilio! We followed up in IRC already, but to add an update here, this is still something we would like to add to the bot. It is one of the most requested features. We are focused on delivering other core features before the end of 2020, so I do not expect we will get it before the end of this year. But I hope we can get it early in 2021.

@Tjzabel Tjzabel moved this from Backlog to Current sprint in TeleIRC development Sep 3, 2021
@Tjzabel
Copy link
Member

Tjzabel commented Mar 29, 2022

Welp, almost two years later this feature is finally getting completed. Currently, the reply quote type + length of reply is configurable, and looks like this by default:

<T​jzabel> Initial text
<T​jzabel> [Re T​jzabel: Initial text] response text

If a message is over 15 characters long, then a "..." is appended after the 15th character.

@robbyoconnor
Copy link
Contributor

If a message is over 15 characters long, then a "..." is appended after the 15th character.

@Tjzabel that works!

@Tjzabel Tjzabel moved this from Current sprint to In progress in TeleIRC development Mar 30, 2022
TeleIRC development automation moved this from In progress to Done May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves on something that already exists Telegram Issues relating to Telegram bridge
Projects
Development

Successfully merging a pull request may close this issue.

6 participants