Skip to content

Add string extensions quote and reverse#998

Open
pkwarren wants to merge 8 commits intogoogle:mainfrom
pkwarren:pkw/string-ext-quote-reverse
Open

Add string extensions quote and reverse#998
pkwarren wants to merge 8 commits intogoogle:mainfrom
pkwarren:pkw/string-ext-quote-reverse

Conversation

@pkwarren
Copy link
Copy Markdown

Add strings.quote and reverse extensions to match Go implementations.

@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 27, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Add `strings.quote` and `reverse` extensions to match Go
implementations.
@pkwarren pkwarren force-pushed the pkw/string-ext-quote-reverse branch from 0904d0d to 75fafc9 Compare March 27, 2026 22:19
@pkwarren
Copy link
Copy Markdown
Author

I wasn't sure if these were left out of the string extensions intentionally, but these were two that I saw implemented in cel-go but weren't in cel-java. The third one format is much more complex so I didn't tackle that one.

@l46kok
Copy link
Copy Markdown
Collaborator

l46kok commented Mar 27, 2026

I wasn't sure if these were left out of the string extensions intentionally, but these were two that I saw implemented in cel-go but weren't in cel-java. The third one format is much more complex so I didn't tackle that one.

Thanks for the contribution. This was just work we hadn't been able to get to, and should definitely be added in CEL-Java.

I'll review more thoroughly soon but in the meanwhile -- would you mind enabling the conformance tests for strings.quote by removing these lines:

https://github.com/google/cel-java/blob/main/conformance/src/test/java/dev/cel/conformance/BUILD.bazel#L123
https://github.com/google/cel-java/blob/main/conformance/src/test/java/dev/cel/conformance/BUILD.bazel#L152

@pkwarren
Copy link
Copy Markdown
Author

Enabled strings.quote in conformance tests in 6207811.

Copy link
Copy Markdown
Collaborator

@l46kok l46kok left a comment

Choose a reason for hiding this comment

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

Please add the newly added function to the documentation: https://github.com/google/cel-java/blob/main/extensions/src/main/java/dev/cel/extensions/README.md?plain=1#L388

You should be able to copy the relevant sections from go's README: https://github.com/google/cel-go/blob/master/ext/README.md

@pkwarren
Copy link
Copy Markdown
Author

Thanks for the thorough review. I believe I've addressed the feedback - only question is on #998 (comment).

Copy link
Copy Markdown
Collaborator

@l46kok l46kok left a comment

Choose a reason for hiding this comment

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

LGTM -- just a few nits

@l46kok l46kok added the ready to pull Pulls the pending PR into Critique label Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to pull Pulls the pending PR into Critique

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants