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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

httpcaddyfile: Add shortcut for expression matchers #4976

Merged
merged 1 commit into from Sep 2, 2022

Conversation

francislavoie
Copy link
Member

@francislavoie francislavoie commented Aug 23, 2022

This is admittedly a bit of a wacky idea... but it's super cool!!

Since #4715 we have support for calling most matchers from within a CEL expression, I think we should also have a nicer syntax for defining a matcher with an expression. Typing out the word expression to get at this functionality is not as nice as it could be.

My first thought was maybe we make an alias like expr? That is better (6 less chars to type) but it still doesn't feel great.

Then I thought of the change we semi-recently made to the parser in #4643 which allows us to detect if a token was explicitly quoted in the config. Matcher names (i.e. the module name) should never be quoted (because... why?) so I think we can make the assumption that if we see something in the place of a matcher name that is quoted, it's a CEL expression.

This reads quite naturally, because the only requirement is that you quote the CEL expression as a token, and it just works! It's true that quoting can be omitted for the long-form because of #4643, but explicitly quoting makes the shortcut possible!

Before:

@notFound expression {err.status_code} == 404
respond @notFound "Oops, not found"

After (also works with " quotes, and even multiline expressions work):

@notFound `{err.status_code} == 404`
respond @notFound "Oops, not found"

Pretty sweet! 馃殌

The main concern is... are we shooting ourselves in the foot with making this assumption? Will we want to do something else with this case at some point in time? I think probably not. I think it would be pretty cool to push CEL expressions forwards as a more first-party feature rather than some "escape-hatch" feeling thing.

And this is fully backwards compatible, because this was previously impossible syntax since the first token was normally supposed to always be a matcher name.

@francislavoie francislavoie added the feature 鈿欙笍 New feature or request label Aug 23, 2022
@francislavoie francislavoie added this to the v2.6.0-beta.1 milestone Aug 23, 2022
@francislavoie francislavoie force-pushed the expression-matcher-shortcut branch 2 times, most recently from bdc4aa2 to a313bc6 Compare August 23, 2022 06:41
@francislavoie
Copy link
Member Author

FYI @TristonianJones 馃槉

@mholt
Copy link
Member

mholt commented Sep 1, 2022

I like it.

Can we document it as experimental? 馃槄 Just to see how it goes, since it kind of came out of nowhere.

But it's way cool. I can imagine this:

@goingDown vars {http.shutting_down} true
respond @goingDown 503

Turning into:

@goingDown `{http.shutting_down}`
respond @goingDown 503

being very elegant! (With the slight oddity that the placeholder looks like a literal-quoted string, but is in fact expanded.)

@francislavoie
Copy link
Member Author

Yeah, we can document it as experimental on the website for now 馃憤

@francislavoie francislavoie merged commit 7d5108d into master Sep 2, 2022
@francislavoie francislavoie deleted the expression-matcher-shortcut branch September 2, 2022 03:12
@mholt mholt modified the milestones: v2.6.0-beta.1, v2.6.0 Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 鈿欙笍 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants