From 498377a230b8712792a47a61ad63ab746f7379d1 Mon Sep 17 00:00:00 2001 From: Duco van Amstel Date: Sat, 13 Oct 2018 00:02:14 +0100 Subject: Clean up code and strengthening (slack) (#519) Changes include: - Refactor of strings into package-wide constants. - Predeclaration of regexps to be instantiated at package load time. - Checking of unchecked errors. - Structural changes: - Adding verifications to type-casting code. - Remove unnecessary 'len(X) > 0' checks before iterating over X. - Remove unnecessary 'else' clause after 'if' with 'return'. - Unexporting of public fields of Bridge struct. - Formatting: - One-field-per-line struct definitions. --- bridge/slack/helpers.go | 73 ++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 38 deletions(-) (limited to 'bridge/slack/helpers.go') diff --git a/bridge/slack/helpers.go b/bridge/slack/helpers.go index 273cf7c6..9af5dfac 100644 --- a/bridge/slack/helpers.go +++ b/bridge/slack/helpers.go @@ -8,8 +8,8 @@ import ( "github.com/nlopes/slack" ) -func (b *Bslack) userName(id string) string { - for _, u := range b.Users { +func (b *Bslack) getUsername(id string) string { + for _, u := range b.users { if u.ID == id { if u.Profile.DisplayName != "" { return u.Profile.DisplayName @@ -17,22 +17,26 @@ func (b *Bslack) userName(id string) string { return u.Name } } + b.Log.Warnf("Could not find user with ID '%s'", id) return "" } func (b *Bslack) getAvatar(userid string) string { - var avatar string - if b.Users != nil { - for _, u := range b.Users { - if userid == u.ID { - return u.Profile.Image48 - } + for _, u := range b.users { + if userid == u.ID { + return u.Profile.Image48 } } - return avatar + return "" +} + +func (b *Bslack) getChannel(channel string) (*slack.Channel, error) { + if strings.HasPrefix(channel, "ID:") { + return b.getChannelByID(strings.TrimPrefix(channel, "ID:")) + } + return b.getChannelByName(channel) } -/* func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) { if b.channels == nil { return nil, fmt.Errorf("%s: channel %s not found (no channels found)", b.Account, name) @@ -44,7 +48,6 @@ func (b *Bslack) getChannelByName(name string) (*slack.Channel, error) { } return nil, fmt.Errorf("%s: channel %s not found", b.Account, name) } -*/ func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) { if b.channels == nil { @@ -58,45 +61,40 @@ func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) { return nil, fmt.Errorf("%s: channel %s not found", b.Account, ID) } -func (b *Bslack) getChannelID(name string) string { - idcheck := strings.Split(name, "ID:") - if len(idcheck) > 1 { - return idcheck[1] - } - for _, channel := range b.channels { - if channel.Name == name { - return channel.ID - } - } - return "" -} +var ( + mentionRE = regexp.MustCompile(`<@([a-zA-Z0-9]+)>`) + channelRE = regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`) + variableRE = regexp.MustCompile(``) + urlRE = regexp.MustCompile(`<(.*?)(\|.*?)?>`) +) // @see https://api.slack.com/docs/message-formatting#linking_to_channels_and_users func (b *Bslack) replaceMention(text string) string { - results := regexp.MustCompile(`<@([a-zA-Z0-9]+)>`).FindAllStringSubmatch(text, -1) - for _, r := range results { - text = strings.Replace(text, "<@"+r[1]+">", "@"+b.userName(r[1]), -1) + replaceFunc := func(match string) string { + userID := strings.Trim(match, "@<>") + if username := b.getUsername(userID); userID != "" { + return "@" + username + } + return match } - return text + return mentionRE.ReplaceAllStringFunc(text, replaceFunc) } // @see https://api.slack.com/docs/message-formatting#linking_to_channels_and_users func (b *Bslack) replaceChannel(text string) string { - results := regexp.MustCompile(`<#[a-zA-Z0-9]+\|(.+?)>`).FindAllStringSubmatch(text, -1) - for _, r := range results { - text = strings.Replace(text, r[0], "#"+r[1], -1) + for _, r := range channelRE.FindAllStringSubmatch(text, -1) { + text = strings.Replace(text, r[0], "#"+r[1], 1) } return text } // @see https://api.slack.com/docs/message-formatting#variables func (b *Bslack) replaceVariable(text string) string { - results := regexp.MustCompile(``).FindAllStringSubmatch(text, -1) - for _, r := range results { + for _, r := range variableRE.FindAllStringSubmatch(text, -1) { if r[2] != "" { - text = strings.Replace(text, r[0], "@"+r[2], -1) + text = strings.Replace(text, r[0], "@"+r[2], 1) } else { - text = strings.Replace(text, r[0], "@"+r[1], -1) + text = strings.Replace(text, r[0], "@"+r[1], 1) } } return text @@ -104,12 +102,11 @@ func (b *Bslack) replaceVariable(text string) string { // @see https://api.slack.com/docs/message-formatting#linking_to_urls func (b *Bslack) replaceURL(text string) string { - results := regexp.MustCompile(`<(.*?)(\|.*?)?>`).FindAllStringSubmatch(text, -1) - for _, r := range results { + for _, r := range urlRE.FindAllStringSubmatch(text, -1) { if len(strings.TrimSpace(r[2])) == 1 { // A display text separator was found, but the text was blank - text = strings.Replace(text, r[0], "", -1) + text = strings.Replace(text, r[0], "", 1) } else { - text = strings.Replace(text, r[0], r[1], -1) + text = strings.Replace(text, r[0], r[1], 1) } } return text -- cgit v1.2.3