add support for raw string using backticks #26
Conversation
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
|
Ditto with making sure that a test case in the printer package handles multiline raw strings properly. |
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
| @@ -0,0 +1,11 @@ | |||
| block "label" { | |||
There was a problem hiding this comment.
labels get printed out with double quotes instead of the original backticks. I think that's probably fine for labels but I wanted to call attention to it.
erikbaranowski
changed the title
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
printer/testdata/raw_string.in
Outdated
| block `label` { | ||
| attr = `'\"attr` | ||
| } |
There was a problem hiding this comment.
Hm, I wonder if we should even support this for labels, or if labels should require the double quoted syntax to start with.
There was a problem hiding this comment.
IMO the parser should reject using raw strings for block labels for now.
There was a problem hiding this comment.
Mostly LGTM, just one final comment about block labels. There will be a few things to clean up after this gets merged:
- We should probably cut a 0.2.0.
- Update the grammar used in grafana.com/docs to include knowledge of raw strings.
- Update the grammar in rfratto/vscode-river and rfratto/vim-river (I really need to move those repos still) to support raw strings.
- Bump the dependency in the agent and document the new string form.
- We should open a feature enhancement in the agent for the UI to support raw strings and render a string as a raw string whenever it's appropriate (like if it has a newline character).
Thanks for working on this! This will save a lot of people headaches.
printer/testdata/raw_string.in
Outdated
| block `label` { | ||
| attr = `'\"attr` | ||
| } |
There was a problem hiding this comment.
IMO the parser should reject using raw strings for block labels for now.
add testing pattern for parsing river with errors Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
| f, err := parser.ParseFile(t.Name()+".rvr", inputBB) | ||
| if expectedError := getExpectedErrorMessage(t, expectErrorFile); expectedError != "" { | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), expectedError) |
There was a problem hiding this comment.
Doing a contains for now so we can test the interesting part of the error. This could become an Equal at the expense of the tests being a little more brittle.
| require.Equal(t, trimmed, buf.String(), "%s", buf.String()) | ||
| } | ||
|
|
||
| // getExpectedErrorMessage will retrieve an optional expected error message for the test. | ||
| func getExpectedErrorMessage(t *testing.T, errorFile string) string { |
There was a problem hiding this comment.
Current implementation of diag.Diagnostics prints out (and %d more diagnostics) so for this testing we are able to validate the first Diagnostic only. This is why this returns string instead of []string like the agent converter tests with an error message per line in the testdata file.
Closes #24