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.

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

Unable to generate HTML formatted messages using the EventFormattingAgent (interpolate_jsonpaths) #255

Closed
dsander opened this issue Apr 22, 2014 · 21 comments

Comments

@dsander
Copy link
Collaborator

dsander commented Apr 22, 2014

I want to use the BasecampAgent to send messages to the HipchatAgent. To format the messages I am currently using the EventFormattingAgent which works great for normal text messages.
HipChat however allows (some) HTML tags to format the message. Because JSONPaths are escaped with angle brackets it is not possible to generate HTML message (or XML/E-Mail address with the full name in front of it).

I have got three solutions in my mind.

  • change the escaping of the JSONPaths to something not commonly used like {{$.message}}
  • allow escaping of opening angle brackets like \<br>
  • use erb to format the mesage <%= payload.message %>

Using erb would add the option to use conditional formatting without adding rules, but could also over complicate the configuration of agents.
My personal favorite would be the first option. If one does not want to generate handlebar templates, I can not imagine a use case where {{ would collide with any commonly used format.

@snicker
Copy link
Contributor

snicker commented Apr 22, 2014

Would it be more appropriate to have a more robust formatting agent? HtmlFormattingAgent or some sort of TemplatingAgent?

@snicker
Copy link
Contributor

snicker commented Apr 22, 2014

I only mention this because if you change the tags, now everyone's installation is broken because their JSONPaths are no longer evaluated and anywhere this type of escaping is used needs to be changed.

@dsander
Copy link
Collaborator Author

dsander commented Apr 22, 2014

True, that would be another option. But as I was testing the Basecamp and Hipchat agents I kind of wanted to be able to use the JSONPath interpolation in the Hipchat agent right away and thus remove the need for the FormattingAgent (which of course will not work for every agent combination but could simplify the configuration for some).
As explanation, in my case the Hipchat agent just needs to format the message which is send to the channel, being able to use the interpolation where like <$.summary> <$.excerpt> <$.html_url> would be enough.

We could just add a migration to change the tags on existing installations to the new format. Search for <[^>](+)> and replace it with {{$1}}.

@snicker
Copy link
Contributor

snicker commented Apr 22, 2014

why not double escape? <<html>>

@dsander
Copy link
Collaborator Author

dsander commented Apr 22, 2014

That would work too, however it feels wrong to me to escape the "established" format and use it's tags in the for the "new" interpolation feature. But that view is of course biased as i am new to huginn and you probably never planned the JSONPath interpolating as some sort of templating system.

Thinking about it and the simplicity of the required changes, I am beginning to like the double escape though 😄

@snicker
Copy link
Contributor

snicker commented Apr 22, 2014

Perhaps the man with the vision @cantino could weigh in with an opinion ;)

@snicker
Copy link
Contributor

snicker commented Apr 22, 2014

better yet... why not Markdown support?

Add a flag on the agent evaluate_markdown and the output agent has proper html output

@dsander
Copy link
Collaborator Author

dsander commented Apr 22, 2014

Hah! Another option, even though it would require the user to learn another language (implying that most of them are familiar with html)

But! When every emitting agent provided a sample event we could create a live preview of the formatted event, which would be awesome

@cantino
Copy link
Member

cantino commented Apr 22, 2014

Hey @dsander and @snicker, thanks for talking this through.

I agree that we need to allow HTML in events, so we can't deny < and > completely. I sort of made up that syntax and am now regretting not using {{ ... }}, or something else equally rare.

@dsander, I think your idea of doing a migration is a pretty good one. We'd need to make a list of all fields that currently support JSONPath, and just migrate those.

We should also allow escaping, as you suggested, so that {{ is a literal {{.

Going forward, I think it'd be awesome if all string fields supported JSONPath by default. It's also likely that the set of valid commands inside of {{}} blocks will expand with time. We already allow {{escape $.url}}. Perhaps another valid command would be {{credential foo}} or similar.

(Basically, this is similar to jinja2.)

What do you folks think?

@snicker
Copy link
Contributor

snicker commented Apr 22, 2014

in an ideal world, {{ }} would be preferred I think. If the change can be accommodated with a migration, then I am all for it. Need some testing on the migration is all!

@dsander
Copy link
Collaborator Author

dsander commented Apr 22, 2014

Ok, I will see how far i can get with the migration and open a PR once there is something to show/test/review

@cantino This is interesting ansible uses a pretty similar syntax as angular in it's filters, maybe it would be worth to change the <escape $.stuff> to something like {{$.stuff | escape}} while we are at it?

@cantino
Copy link
Member

cantino commented Apr 22, 2014

Maybe. I do like the piping syntax.

In fact I've been playing with something similar:

screen shot 2014-03-24 at 11 38 29 pm

@mfclarke
Copy link
Contributor

+1 for markdown support. I think that would be killer for the email agents too

@mfclarke
Copy link
Contributor

Actually just re-reading, html and jinja style variables would be awesome. Don't mind me, as you were ;)

@dsander
Copy link
Collaborator Author

dsander commented Apr 23, 2014

I have looked at some existing templating systems for ruby and came across liquid, which looks like a perfect fit for us. What do you guys think?

@cantino
Copy link
Member

cantino commented Apr 24, 2014

Liquid is quite cool, and might be the right solution, as long as {% is properly escaped so that literal {% tokens can be expressed too. Another option is mustache. Any opinions? (/cc @albertsun @knu)

@knu
Copy link
Member

knu commented Apr 24, 2014

I would prefer Liquid in the two because it has an extensible syntax and less cryptic.
Both has a node implementation, so it could be easy to make it available in JavaScriptAgent also.

@mfclarke
Copy link
Contributor

Liquid looks great, seems easy to follow and I like the piping syntax (though I'm probably biased, I've done a bit of Django stuff in the past and it's template files use a similar syntax).

Mustache looks kind of ugly and awkward to follow, but that could simply be because I've never used it

@dsander
Copy link
Collaborator Author

dsander commented Apr 24, 2014

I looked at mustache too, but I am really excited about the pipe syntax in liquid. About escaping {{ and {%, I think you would need to write something like this {{ '{{' }} to have {{ in our output. But that's pretty much the same in every templating engine, right?

@cantino
Copy link
Member

cantino commented Apr 24, 2014

My vote is Liquid. It sounds like that is most people's preference, so let's use that.

@albertsun
Copy link
Contributor

Liquid looks like a good option.

I've used mustache quite a bit and have mixed feelings about it. The syntax seems simpler, but there's a bunch of things that can be quite difficult to do with Mustache like conditionals and logical operators and require code to reformat data beforehand.

@dsander dsander closed this as completed May 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants