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

Add syntax for escaped field selectors. #1002

Merged
merged 5 commits into from
Dec 16, 2024

Conversation

jnthntatum
Copy link
Collaborator

@jnthntatum jnthntatum commented Aug 14, 2024

Introduces syntax for escaped fields that are not normally syntactically legal field selections with backticks.

This slightly relaxes the parse requirements for fields to cover more types of field like keys e.g.

tags.`skewer-case-name` 
proto.`com.example.ext.field_name`
paths.`/api/v1`

This PR is limited to fields, but we could consider generalizing to cover namespaced names and simple identifier for variables in a later update (e.g. in.example.Foo or package)

Copy link
Collaborator

@TristonianJones TristonianJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this as a first approach at relaxing the syntax to support escapes need for fields named after reserved words and for supporting proto extension fields natively.

The type-checker should continue to validate field names which is great, and there's no weirdness with invalid function names, just escaping on the namespaces. It seems to help satisfy a common pain point. It might not solve top-level .in domains within namespace names, but it's a great start.

Introduces syntax for escaped fields that are not normally
syntactically legal field selections with backticks.

This PR is limited to fields, but would also consider adding for namespaced
names (e.g. `in.example.Foo`)
- Reduce set of allowed escaped identifiers
- Additional testing
@jnthntatum jnthntatum changed the title PoC: Add syntax for escaped field selectors. Add syntax for escaped field selectors. Dec 10, 2024
@jnthntatum jnthntatum marked this pull request as ready for review December 10, 2024 21:46
Copy link
Collaborator

@TristonianJones TristonianJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. My questions which can be addressed post-submission.

@jnthntatum jnthntatum merged commit a108e9e into google:master Dec 16, 2024
2 checks passed
@jnthntatum jnthntatum deleted the cel-escaped-idents branch January 7, 2025 20:34
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

Successfully merging this pull request may close these issues.

2 participants