Add custom formatting to IRC & Discord output #204
Conversation
|
I guess I could add a new "describe" context, within the current one, for the tests that use a regular-config bot, and separate out the Also: huh, I really wasn't expecting coverage to decrease that much. Is there some easy way for me to extract out that repeated function in the |
|
I've added tests for this (which should hopefully now pass, making this not a WIP), but perhaps instead of creating a new Opinions? |
Throne3d
force-pushed
the
add/custom-formatting
branch
from
136d6ac to
dfe13b1
Compare
|
Nm, undid the contexting and just manually used |
Throne3d
changed the title
lib/bot.js
Outdated
| @@ -148,6 +168,13 @@ class Bot { | |||
| return this.commandCharacters.indexOf(message[0]) !== -1; | |||
| } | |||
|
|
|||
| static substitutePattern(message, patternMapping) { | |||
| return message.replace(patternMatch, (match, varName) => { | |||
| if (varName in patternMapping) return patternMapping[varName]; | |||
There was a problem hiding this comment.
Could simplify this to (match, varName) => patternMapping[varName] || match?
There was a problem hiding this comment.
The main reason I didn't was in case the patternMapping was a blank string. I guess this probably isn't going to happen anywhere (unless there was somehow a zero-length discord message? But there's a guard for that), so it should be fine to simplify.
lib/bot.js
Outdated
| this.formatCommandPrelude = options.formatCommandPrelude || 'Command sent from Discord by {$nickname}:'; | ||
| this.formatIRCText = options.formatIRCText || '<{$displayUsername}> {$text}'; | ||
| this.formatURLAttachment = options.formatURLAttachment || '<{$displayUsername}> {$attachmentURL}'; | ||
| // "{$keyName}" => "variableValue" |
There was a problem hiding this comment.
Nitpick, but I think it makes more sense to put these comments above the variables.
lib/bot.js
Outdated
| @@ -33,6 +34,25 @@ class Bot { | |||
| this.ircNickColor = options.ircNickColor !== false; // default to true | |||
| this.channels = _.values(options.channelMapping); | |||
|
|
|||
| this.formatCommandPrelude = options.formatCommandPrelude || 'Command sent from Discord by {$nickname}:'; | |||
There was a problem hiding this comment.
Maybe create a format object and have these as properties? I.e.
"format": {
"commandPrelude": "...",
"ircText": "...",
"urlAttachment": "..."
}or something like that? Then just add a this.format = options.format || {}; above?
|
Looks great, thanks! I've normally been against adding too many configuration options, but I think this could be useful. |
Also fix an issue in a previous commit whereby Discord channel names were not prefixed with '#' (despite the comments suggesting that to be the case).
Throne3d
force-pushed
the
add/custom-formatting
branch
from
dfe13b1 to
9848a89
Compare
|
(Rebased onto master, rebased to move comments, updated to work with the new formatting-bridging thing ( |
|
Released in 2.3.0! |
As requested by #13, it might sometimes be useful to have custom formatting in the output for IRC and Discord.
I'm not sure how exactly to add tests for this, as it seems like it's going to need a separate configuration for (almost) each test, and this doesn't look like it'll fit into the current
bot.test.jsfile very easily. From a brief glance I can't see a 'context' feature in Chai as there is in RSpec, which would allow me to limit thebeforeEachto just the tests present, and add separate ones at the end for this. It's possible I'm missing something.I'm also not too strongly attached to this idea and it might make it harder to implement some things in the future, so if it'd be preferable to drop this, I don't care too strongly. (If people want it desperately enough, they can always just patch these commits on – hopefully it's clearly documenting enough, and it doesn't seem to break current tests.)
I'd appreciate feedback on how to proceed. Thanks.