New Features: sendFile and sendHtml

[aaaschmitt]

This pr adds some HipChat specific functionality. Check the updated README for usage, but an example script can also be found here.

This pr adds two new functions to the response object that an executed script receives resp.sendHtml and resp.sendFile. There was a bit of a trick to adding these new features as the standard hubot response does not have these features. I extended the hubot Response class with a HipChatResponse object that contains these two new methods. Then when the adapter is initialized it replaces the robot's response object with its own.

Any comments would be greatly appreciated, but I'm sure myself and others would love to see this pulled in as part of the adapter.

[voidfiles]

This is cool. I've pulled your or into my project, and it works great. I am wondering though if it could support more of the optional available in the API.

https://www.hipchat.com/docs/apiv2/method/send_room_notification

I think if you add an additional Param to sendHtml that would be passed to the API along with the hyml message you could support many of those options.

[mattelacchiato]

Is there a reason, why this PR isn't merged yet?

[jonyeezs]

How does this work with other plugins - sendHtml and sendLink looks to be a custom send command.

This won't work well with hooking to other plugins.

I recommend to have some condition settings in the send command.
ie: if there are html tags use sendHtml

[aaaschmitt]

I believe that the hipchat adapter is not really accepting new features anymore so I doubt that this will ever be pulled in. There is also more of a meta discussion (I believe it was somewhere in the slack adapter forum) around whether or not the adapters base functionality should actually be extended like this. The problem being that this then leads to adapters diverging in functionality from each other, which then means that even more scripts will be tied to a particular chat platform.

As of late I have also stopped using hipchat for my work so I am probably not going to be maintaining these features. I highly encourage anyone to pull them into their branches though or use the concepts presented here to make other cool things, particularly how you can attach new functions to the response object!

@jonyeezs That may be true but if you have another plugin that does the same thing then maybe you should ask yourself why you have that plug in? Also do plugins usually modify the adapter interface (maybe they do)? I thought usually you just include scripts which can then only interface through the adapter. In those cases, which is all I have worked with, there will be no collisions since this functionality is in the adapter itself.

I also don't see how what you've proposed is better. There are certainly cases where someone would want to share raw html text in a chat room (particularly if the chat room is among a bunch of programmers) so auto detection here is not a good option. You also haven't addressed how you would avoid collisions with sendFile (I'm guessing you meant to write that instead of sendLink) since having it auto detect resp.send("myfile.txt") I don't think is a good solution.

[jonyeezs]

I agree with what you say.

I don't think my solution is the correct but i suppose was pointing out
that introducing new commands to the adapter that doesn't follow hubot's
interface isn't the right way. Which is basically what you mentioned in the
former

On Fri, 30 Sep 2016, 3:34 PM Andy Schmitt [email protected] wrote:

I believe that the hipchat adapter is not really accepting new features
anymore so I doubt that this will ever be pulled in. There is also more of
a meta discussion (I believe it was somewhere in the slack adapter forum)
around whether or not the adapters base functionality should actually be
extended like this. The problem being that this then leads to adapters
diverging in functionality from each other, which then means that even more
scripts will be tied to a particular chat platform.

As of late I have also stopped using hipchat for my work so I am probably
not going to be maintaining these features. I highly encourage anyone to
pull them into their branches though or use the concepts presented here to
make other cool things, particularly how you can attach new functions to
the response object!

@jonyeezs https://github.com/jonyeezs That may be true but if you have
another plugin that does the same thing then maybe you should ask yourself
why you have that plug in? Also do plugins usually modify the adapter
interface (maybe they do)? I thought usually you just include scripts which
can then only interface through the adapter. In those cases, which is all I
have worked with, there will be no collisions since this functionality is
in the adapter itself.

I also don't see how what you've proposed is better. There are certainly
cases where someone would want to share raw html text in a chat room
(particularly if the chat room is among a bunch of programmers) so auto
detection here is not a good option. You also haven't addressed how you
would avoid collisions with sendFile (I'm guessing you meant to write that
instead of sendLink) since having it auto detect resp.send("myfile.txt") I
don't think is a good solution.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#270 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKXAhh36FZ_rPOZIkgCHs8h6FkOwT_P_ks5qvJ9QgaJpZM4HEmQf
.