Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

kql: only allow top() at start of selector #388

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

alightgoesout
Copy link
Contributor

Following discussion #387, I propose a change to the KQL grammar. With this change, top() will only be allowed at the start of a KQL selector.

@zkat
Copy link
Member

zkat commented Apr 15, 2024

Maybe we can add some explanatory text as well, to clarify that top() is toplevel only, not just put it in the grammar?

@alightgoesout
Copy link
Contributor Author

Yes you're right, I have added an explanation.

@zkat zkat requested a review from larsgw April 16, 2024 17:01
@zkat
Copy link
Member

zkat commented Apr 16, 2024

lgtm, I'll let @larsgw have an eyeball at it since he was part of our discussion, too.

@larsgw
Copy link
Contributor

larsgw commented Apr 17, 2024

I'd rather have selector and selector-subsequent so that the main selector rule is called selector and not selector-start, that makes it slightly easier to translate to a grammar. Otherwise, lgtm.

@alightgoesout
Copy link
Contributor Author

I have renamed the rules as suggested.

@zkat zkat merged commit 6a77436 into kdl-org:kdl-v2 Apr 17, 2024
@alightgoesout alightgoesout deleted the no_top_inside_kdl branch April 22, 2024 15:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants