velociraptor icon indicating copy to clipboard operation
velociraptor copied to clipboard

A comment between GROUP BY and LIMIT breaks the formatter and markdown renderer

Open misje opened this issue 1 year ago • 2 comments

/*
# This is a Markdown title
*/

// Works just fine:
SELECT 'Foo' FROM scope()

SELECT 'Bar' AS Bar FROM scope()
GROUP BY Bar
// Troublesome comment
ORDER BY Bar

This VQL will execute without any errors, but the auto-formatter will complain with

Error: 11:1: unexpected token "ORDER BY"

and any Markdown in the cell will not render.

misje avatar Jul 14 '24 18:07 misje

Yes that's how it works.

Basically to evaluate vql we just strip the comments and evaluate it so there's no problem with comments anywhere, but to reformat and render md we need to preserve the comments which means we need to parse them into the syntax tree.

So we have to attach the comment to a vql element which means we have to expect a comment at a particular place. There are only certain places in the syntax that may carry a comment. If the comment appears somewhere weird we just don't know what to do with it and how to preserve it.

We can probably add the comment to this particular place in the syntax but in general there will always be weird comment locations that will break things (also multiple comments will not work).

Perhaps this error is about trying to figure out if the syntax is really broken (ie. fails to parse with comments stripped) or is it a comment related issue (i.e. fails to parse with comments not stripped). The error message should be clearer.

scudette avatar Jul 14 '24 23:07 scudette

There are only certain places in the syntax that may carry a comment

I think if these were documented, a number of surprises I've had with the languages and comments could probably be explained. Is there a place this is documented already, or it is possible to get a good overview from the code?

misje avatar Jul 15 '24 05:07 misje

I think there is not much else to do here - the artifact will save properly just not be able to reformat.

scudette avatar Jul 17 '25 14:07 scudette