Support line breaks in alert descriptions

[guoshimin]

From IRC:

jrv: shimin: yeah, I believe we still need to add a way to specify multi-line strings. Either with \n-style interpolation or allowing raw newlines in strings...

[brian-brazil]

For description we'd want raw, and pretty much everywhere else we'd want \n so I guess that means both.

[juliusv]

Looking into this, adding support for raw newlines seems easy (simply don't error on them within a string), but supporting escaped characters is going to be more difficult: the parser just stores the raw part of the input expression that constitutes the string, without doing any escape character interpretation. So if the input expression is

"my\nstring"

...the string node that will be stored will contain the literal value

`"my\nstring"`

...and not

"my\nstring"

We need a mechanism to actually interpret all the possible escape sequences. The escape lexing part we could copy from https://golang.org/src/go/scanner/scanner.go?s=931:1526#L364, but not sure where the actual interpretation happens yet.

[juliusv]

For our purposes, it seems cleanest to still let the lexer store the raw strings (as it does for any token it scans) and then do the escape interpretation in the parser (do a call to expandEscapeSequences() or similar on the raw string val of the node, which can start out simple to just support \n-style newlines).

[juliusv]

@fabxc does that sound good? Then I'd go ahead with that.

[fabxc]

Whatever makes things as we want them to in a clean way.
I would prefer multi-line strings to be python-styled with triple quotes.

On Tue, Sep 29, 2015 at 3:28 PM Julius Volz notifications@github.com
wrote:

@fabxc https://github.com/fabxc does that sound good? Then I'd go ahead
with that.

—
Reply to this email directly or view it on GitHub
#1121 (comment)
.

[brian-brazil]

This is going to (presumably) apply to the parser everywhere, so we probably want to think a little how it'll work on the expression browser etc.

[juliusv]

Yeah, I don't see any issues with that being handled the same everywhere (whether interactive expressions or rules). Ok, I'll get to this soon.

[juliusv]

@fabxc Since our user-exposed templating is already Go-based, wouldn't it be nicer to also use Go-style backticks for raw strings rather than Pythonesque triple quotes? They also need less lookahead logic ("" is a complete string vs. """ being only the beginning of a string.).

The bigger question is whether we then also want to disable \ escape sequences in raw strings like in Go. In Go, you can't even escape backticks in raw string literals, which can be worked around in Go by concatenating multiple strings (not yet possible in Prometheus). I'd go for doing it like in Go for now, since the alternative would also have drawbacks. People can still use normally-quoted single-line strings with \n if they need to include backticks in their strings?

[fabxc]

We have $value and $labels indirection with the comment stating explicitly that we do so because we don't want to expose users to Go specifics. I don't care too much – but only one reasoning should be applied.

[juliusv]

Hm yeah, I guess $value and $labels are still Go-specific, just less scary for the common user than .Value and .Labels :)

Looking at how to implement this, I've found https://golang.org/pkg/strconv/#Unquote, which would be pretty much reusable if we decide to do it the Go way. The exception would be single-quoted strings, which are runes in Go, but normal strings in Prometheus. These could be manually converted on-the-fly into double-quoted strings before applying Unquote to them (or into backticked strings if we wanted to treat them that way).

[fabxc]

We had issues with the Unquote and single quotes in the past. "Pretty much reusable" is out of the window if the takes another three sentences to explain which cases "pretty much" does not cover.
I would not take this as an argument for going any particular direction.

I also don't think we want the rawness-behavior for those but really just the addition of multi-lines.

[juliusv]

The issue with Unquote was exactly the single-quote handling: #685

But that introduced the bug that we have now (including tests that expect broken output), namely that escape sequences no longer work. I would rather not try to reimplement all the logic in Unquote(), as I think it's much more likely to get a bug that way than by working around this one special case in Unquote().

[juliusv]

Correction: the tests added in #685 were expecting the right thing, as that's in the lexer and still expected to be raw. But we'll need additional tests for the parser now to make sure that the escape sequences get handled correctly on that level.

[prat0318]

I went throught the commit, but still couldn't figure out how should i put the \n in the description to show in slack. Can we please mention it here.

[juliusv]

@prat0318 Strings work basically like in Go. If you want newlines, you can either put \n in a double-quoted string or have an actual newline in a backtick-quoted string, like so:

`foo
bar`

If that isn't visible in Slack, is it maybe because Slack expects HTML or something? Like a <br>?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.