Strip formatting better, use new SlackTextMessage class

[lilyball]

Rewrite the removeFormatting method to remove formatting in a slightly
better way:

• Uses one pass instead of two for link handling

• Displays just the label instead of "#{link} #{label}" for regular
  links with labels. I don't know why it was using both, that produces
  weird output like

  apple.com http://apple.com

• Removes entities, which the old version didn't even try to handle

Add a new exported SlackTextMessage class that contains both the parsed
message (as @text) and the "raw" message (as @rawText). This provides an easy
way for scripts to detect that the message came from the Slack adapter and
handle the unparsed message if so desired.

[paulhammond]

Hi Kevin, thanks for sharing your code! Couple of thoughts on this one...

Firstly, displaying just the label instead of #{link} #{label} is specifically not what we want to do here. You're assuming that all links sent from slack look like <http://www.example.com|example.com>, but sometimes they actually look like <http://www.example.com|label that doesn't include the domain name at all>. Turning that into label that doesn't include the domain name at all means any hubot script that is looking for URLs will miss those messages.

Unfortunately, to handle that case then it seemed to me that doing two passes makes the code clearer - the alternative is to have the label special case repeated inside the switch statement. I was 50:50 on which way was better when I originally wrote the code, I could see an argument for moving to one pass if you feel strongly that it's better.

I do think it's a bug that we don't ignore the label if it's a substring of the URL, I should have caught that one earlier, but we could handle this with a check against the label inside the formatting code.

It's also a bug that we weren't decoding <>&, so thanks for pointing that out. But I'd hope we could update the unit tests to match, and check for some obvious edge cases (eg: ampersands inside urls, angle brackets in labels).

Lastly, just a style note, but making one pull request containing one commit containing 2 unrelated changes makes it harder to merge the code. The SlackTextMessage bit is really good and would have been merged immediately if it was separate 😄

[lilyball]

I'll submit an update later that makes it check if the label is a substring. Otherwise, I'd say it should at least wrap the URL in parentheses.

As for two unrelated changes, I was originally moving the parsing code into the new message class, but I changed my mind. That's why I ended up with both changes in the same commit.

[paulhammond]

I just posted #131 which contains all of the updates to the formatting code we've discussed here (including the parenthesis idea!)

It doesn't yet contain the SlackTextMessage change - i'm happy to make that change, or I'm happy to take a pull request, whatever is best for you?

[lilyball]

I'll submit a pull request for it later when I'm back at home. I have some other related changes I'm working on (e.g. bot_message) so I'll submit it with those.

[paulhammond]

sounds great! closing this one out until then!