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.

Sign up for GitHub

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

Jump to bottom

Formatting options feature #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Formatting options feature #76

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52df73b
Initial support for message formatting
llmII Feb 12, 2021
5f36d24
Fix: Using wrong event codes for formatting
llmII Feb 28, 2021
5fb6cc2
Merge branch 'master' into formatting-options-feature
qaisjp Mar 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bridge/bridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ type Config struct {
IRCFilteredMessages []glob.Glob
DiscordFilteredMessages []glob.Glob

// formatters
DiscordFormat map[string]string // formatting for non-PRIVMSG relays from IRC to Discord
IRCFormat string // format for messages relayed in simple mode

// NoTLS constrols whether to use TLS at all when connecting to the IRC server
NoTLS bool

Expand Down
9 changes: 6 additions & 3 deletions bridge/irc_connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,15 @@ func (i *ircConnection) OnPrivateMessage(e *irc.Event) {
}

d := i.manager.bridge.discord
msg := i.manager.formatDiscordMessage("PM", e, e.Message(), "")

// if we have an empty message
if msg == "" {
return // do nothing, Discord doesn't like those
}

i.introducePM(e.Nick)

msg := fmt.Sprintf(
"%s,%s - %s@%s: %s", e.Connection.Server, e.Source,
e.Nick, i.manager.bridge.Config.Discriminator, e.Message())
_, err := d.ChannelMessageSend(i.pmDiscordChannel, msg)
if err != nil {
log.Warnln("Could not send PM", i.discord, err)
Expand Down
44 changes: 26 additions & 18 deletions bridge/irc_listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package bridge

import (
"fmt"
"strings"

ircf "github.com/qaisjp/go-discord-irc/irc/format"
Expand Down Expand Up @@ -67,12 +66,17 @@ func (i *ircListener) OnNickRelayToDiscord(event *irc.Event) {
return
}

oldNick := event.Nick
newNick := event.Message()
message := i.bridge.ircManager.formatDiscordMessage("NICK", event, newNick, "")

// if the message is empty...
if message == "" {
return // do nothing, Discord doesn't like empty messages anyway
}

msg := IRCMessage{
Username: "",
Message: fmt.Sprintf("_%s changed their nick to %s_", oldNick, newNick),
Message: message,
}

for _, m := range i.bridge.mappings {
Expand Down Expand Up @@ -136,29 +140,33 @@ func (i *ircListener) OnJoinQuitCallback(event *irc.Event) {
return
}

who := event.Nick
message := event.Nick
id := " (" + event.User + "@" + event.Host + ") "
message := ""
content := ""
target := ""
manager := i.bridge.ircManager

switch event.Code {
case "STJOIN":
message += " joined" + id
message = manager.formatDiscordMessage("JOIN", event, "", "")
case "STPART":
message += " left" + id
if len(event.Arguments) > 1 {
message += ": " + event.Arguments[1]
content = event.Arguments[1]
}
message = manager.formatDiscordMessage("PART", event, content, "")
case "STQUIT":
message += " quit" + id

reason := event.Nick
content := event.Nick
if len(event.Arguments) == 1 {
reason = event.Arguments[0]
content = event.Arguments[0]
}
message += "Quit: " + reason
message = manager.formatDiscordMessage("QUIT", event, content, "")
case "KICK":
who = event.Arguments[1]
message = event.Arguments[1] + " was kicked by " + event.Nick + ": " + event.Arguments[2]
target, content = event.Arguments[1], event.Arguments[2]
message = manager.formatDiscordMessage("KICK", event, content, target)
}

// if the message is empty...
if message == "" {
return // do nothing, Discord doesn't like empty messages anyway
}

msg := IRCMessage{
Expand All @@ -173,10 +181,10 @@ func (i *ircListener) OnJoinQuitCallback(event *irc.Event) {
channel := m.IRCChannel
channelObj, ok := i.Connection.GetChannel(channel)
if !ok {
log.WithField("channel", channel).WithField("who", who).Warnln("Trying to process QUIT. Channel not found in irc listener cache.")
log.WithField("channel", channel).WithField("who", event.Nick).Warnln("Trying to process QUIT. Channel not found in irc listener cache.")
continue
}
if _, ok := channelObj.GetUser(who); !ok {
if _, ok := channelObj.GetUser(event.Nick); !ok {
continue
}
msg.IRCChannel = channel
Expand Down
36 changes: 29 additions & 7 deletions bridge/irc_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,18 @@ func (m *IRCManager) generateNickname(discord DiscordUser) string {
return newNick
}

func (m *IRCManager) formatIRCMessage(message *DiscordMessage, content string) string {
msg := m.bridge.Config.IRCFormat
length := len(message.Author.Username)
msg = strings.ReplaceAll(msg, "${USER}", message.Author.Username[:1]+"\u200B"+message.Author.Username[1:length])
msg = strings.ReplaceAll(msg, "${DISCRIMINATOR}", message.Author.Discriminator)
msg = strings.ReplaceAll(msg, "${CONTENT}", content)

// we don't do trimming and later checks here, IRC doesn't mind blank messages or at least doesn't complain
// as loudly as Discord
return msg
}

// SendMessage sends a broken down Discord Message to a particular IRC channel.
func (m *IRCManager) SendMessage(channel string, msg *DiscordMessage) {
if m.ircIgnoredDiscord(msg.Author.ID) {
Expand All @@ -382,14 +394,8 @@ func (m *IRCManager) SendMessage(channel string, msg *DiscordMessage) {

// Person is appearing offline (or the bridge is running in Simple Mode)
if !ok {
length := len(msg.Author.Username)
for _, line := range strings.Split(content, "\n") {
m.bridge.ircListener.Privmsg(channel, fmt.Sprintf(
"<%s#%s> %s",
msg.Author.Username[:1]+"\u200B"+msg.Author.Username[1:length],
msg.Author.Discriminator,
line,
))
m.bridge.ircListener.Privmsg(channel, m.formatIRCMessage(msg, line))
Copy link
Owner

Choose a reason for hiding this comment

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

Can we make m.formatIRCMessage return a slice (so that it only needs to be passed msg)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean move m.bridge.ircListener.Privmsg(channel, m.formatIRCMessage(msg, line)) out of the "for range" and have it just take msg? Probably better to do it that way actually, instead of formatting per channel. Could you explain what you mean by return a slice however?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, rereading the code, but basically the for range wrapping this splits the line on '\n' and that's why I made it take a line argument. The function is to format lines, not to split lines out and format them. That said the formatting works on things besides lines anyway so still moving that outside of the for range and letting it then be split on '\n' works too I would think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking through this more, it wouldn't work, formatIRCMessage would need to be aware of new lines '\n' and apply each line as content.

Say the format is "\x02${USER}\x02 ${CONTENT}" - if we treat the entire msg.Content as the singular content then only the first line will have the "\x02${USER}\x02" part and the rest of the lines will have no prefix of the user.

That or I'm misunderstanding what is wanted to be done...

The reason formatIRCMessage needs to remain unaware that it may be handling more than one line is if I add support for including the referenced content in the message (for edit/reply) then this will also be applying for IRCConnections when pushing into their messages channel.

}
return
}
Expand Down Expand Up @@ -427,6 +433,22 @@ func (m *IRCManager) SendMessage(channel string, msg *DiscordMessage) {
}
}

func (m *IRCManager) formatDiscordMessage(msgFormat string, e *irc.Event, content string, target string) string {
msg := ""
if format, ok := m.bridge.Config.DiscordFormat[strings.ToLower(msgFormat)]; ok && format != "" {
msg = format
msg = strings.ReplaceAll(msg, "${NICK}", e.Nick)
msg = strings.ReplaceAll(msg, "${IDENT}", e.User)
msg = strings.ReplaceAll(msg, "${HOST}", e.Host)
msg = strings.ReplaceAll(msg, "${CONTENT}", content)
msg = strings.ReplaceAll(msg, "${TARGET}", target)
msg = strings.ReplaceAll(msg, "${SERVER}", e.Connection.Server)
msg = strings.ReplaceAll(msg, "${DISCRIMINATOR}", m.bridge.Config.Discriminator)
}

return strings.Trim(msg, " ")
}

// RequestChannels finds all the Discord channels this user belongs to,
// and then find pairings in the global pairings list
// Currently just returns all participating IRC channels
Expand Down
50 changes: 50 additions & 0 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"flag"
"fmt"
"os"
"os/signal"
"path/filepath"
Expand Down Expand Up @@ -94,10 +95,21 @@ func main() {
//
viper.SetDefault("irc_puppet_prejoin_commands", []string{"MODE ${NICK} +D"})
ircPuppetPrejoinCommands := viper.GetStringSlice("irc_puppet_prejoin_commands") // Commands for each connection to send before joining channels
rawDiscordFormat := viper.GetStringMapString("discord_format")
var discordFormat map[string]string
if df, err := setupDiscordFormat(rawDiscordFormat); err == nil {
discordFormat = df
} else {
log.WithError(err).Fatal("discord_format setting is invalid")
return
}
//
viper.SetDefault("avatar_url", "https://ui-avatars.com/api/?name=${USERNAME}")
avatarURL := viper.GetString("avatar_url")
//
viper.SetDefault("irc_format", "<${USER}#${DISCRIMINATOR}> ${CONTENT}")
ircFormat := viper.GetString("irc_format")
//
viper.SetDefault("irc_listener_name", "~d")
ircUsername := viper.GetString("irc_listener_name") // Name for IRC-side bot, for listening to messages.
// Name to Connect to IRC puppet account with
Expand Down Expand Up @@ -142,7 +154,9 @@ func main() {
AvatarURL: avatarURL,
Discriminator: discriminator,
DiscordBotToken: discordBotToken,
DiscordFormat: discordFormat,
GuildID: guildID,
IRCFormat: ircFormat,
IRCListenerName: ircUsername,
IRCServer: ircServer,
IRCServerPass: ircPassword,
Expand Down Expand Up @@ -227,6 +241,14 @@ func main() {
}
dib.Config.DiscordIgnores = discordIgnores

rawDiscordFormat := viper.GetStringMapString("discord_format")
if discordFormat, err := setupDiscordFormat(rawDiscordFormat); err == nil {
dib.Config.DiscordFormat = discordFormat
} else {
log.WithError(err).Error("discord_format setting is invalid, this setting has not been updated")
}
dib.Config.IRCFormat = viper.GetString("irc_format")

chans := viper.GetStringMapString("channel_mappings")
equalChans := reflect.DeepEqual(chans, channelMappings)
if !equalChans {
Expand Down Expand Up @@ -282,6 +304,34 @@ func setupFilter(filters []string) []glob.Glob {
return matchers
}

func setupDiscordFormat(discordFormat map[string]string) (map[string]string, error) {
var err error
// lowercase to match that YAML lowercases it
discordFormatDefaults := map[string]string{
"pm": "${SERVER},${NICK}!${IDENT}@${HOST} - ${NICK}@${DISCRIMINATOR}: ${CONTENT}",
"join": "_${NICK} joined (${IDENT}@${HOST})_",
"part": "_${NICK} left (${IDENT}@${HOST}) - ${CONTENT}_",
"quit": "_${NICK} quit (${IDENT}@${HOST}) - Quit: ${CONTENT}_",
"kick": "_${TARGET} was kicked by ${NICK} - ${CONTENT}_",
"nick": "_${NICK} changed nick to ${CONTENT}_",
}
Comment on lines +313 to +320
Copy link
Owner

Choose a reason for hiding this comment

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

I'd still like the default to be off (empty string), so maybe we should move this to config.yml, but commented out.

Additionally, please could you try to match the original format that was created programmatically, if possible? The original ones were designed to match irccloud's output 1:1, which I believe to be a reasonable default for good UX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I hate typing all caps and special symbols pretend the following are the interpolators...

$n == ${NICK}
$d == ${DISCRIMINATOR}
$i == ${IDENT}
$h == ${HOST}
$t == ${TARGET}
$c == ${CONTENT}
$s == ${SERVER}

Now may be a good time to discuss what we want the interpolators to actually be. I'm not picky, and seeing as I only must type them once, the super-long-all-caps ones are okay with me. Discussing them and having to reference them is a pain though.

For join, part, quit, kick, nick I can definitely supply commented out config.yml entries for them that match the original ones before formatting. I actually think the only thing I changed was adding in surrounding '_' for italics and possibly changing the separator between content and metadata to be '-' (I always find that more visually appealing, the '-', and I'm going to at this moment guess I changed that though it may be part of mirroring the original).

For pm we run into an issue. I see 3 options.

  1. Drop metadata ($i$h$s) out and use "$n@$d: $c"
  2. Keep and find a way to format all the data that is appealing and makes it easy for a person to figure out how to reply...
  3. Complicate PM parsing and create a new way for discord users to reply that is compliant with whatever we come up with here as the default.

So, 1st question - Do you want PM to default to "" (empty) (I don't think so but good to figure this out now).

An example of 2 and why it leads to 3 is... consider "$n/$d!$i@$h: $c"... Looks like a good idea but I've seen pylinks use '/' in nicks. So even formatting appealingly will lead to 3 unless we pick a symbol that is impossible to be found in a hostmask to set as a seperator between $n and $d and from there we need to change PM reply parsing to match so it's a simple "copy/paste" vs having to type and pick and figure out which part is which. If we went with '/' then PM reply parsing would need a heuristic or a "try few different ways until truly fail" both of which would lead to suprises when you have "llmII/pylink/actual_network!ident@host" and "llmII/same_actual_network!ident@host". I'd also argue for dropping out $s (why would that be needed any more with network discriminators?) $s has the potential to leak information if the puppet connects to a "hidden" server over the public internet to some degree.

Copy link
Owner

@qaisjp qaisjp Mar 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

In what way are you personally planning to use the PM format? It seems like an outlier, and does not seem to be necessary for configuration, from my point of view.


for ev, format := range discordFormatDefaults {
if df, ok := discordFormat[ev]; !ok || df == "" {
discordFormat[ev] = format
}
}

for ev := range discordFormat {
if _, ok := discordFormatDefaults[ev]; !ok {
err = fmt.Errorf("Unknown format key %s", ev)
break
}
}

return discordFormat, err
}

func SetLogDebug(debug bool) {
logger := log.StandardLogger()
if debug {
Expand Down