-
-
Notifications
You must be signed in to change notification settings - Fork 195
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 ExprTrait
(#771)
#791
Add ExprTrait
(#771)
#791
Conversation
Oh wow, thank you for coming back! This looks really neat! I would say, there are two options going forward, and also depends on you choice:
|
Ok, if I'm too late for SeaORM 1.0, then I'll go with the backwards-compatible path now. I'll finish with this PR first, then I explore the opportunities to UPD: the notes are at #795. To finish this PR, I'm going to:
|
I reverted the breaking impls and updated the PR description. I'll add the documentation in the following days, and then the PR should be ready to merge. |
I've added documentation/doctests. The PR should be ready to merge now. The docs are copy-pasted from the original inherent methods of And I've noticed that the doctest for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
🎉 Released In 0.32.0-rc.2 🎉Thank you everyone for the contribution! |
🎉 Released In 0.32.0 🎉Thank you everyone for the contribution! |
Edited PR Info
Expr
andSimpleExpr
, which now simply forward toExprTrait
methods. I call it small, because all signatures are exactly the same and the compiler will simply suggest addinguse sea_query::ExprTrait
, which is enough to fix the breakage.impl ExprTrait for C: ColumnTrait
insea_orm
.ColumnTrait
methods. Their signatures don't match exactly, so there's going to be more breakage than in case ofsea_query
.New Features
trait ExprTrait
Expr
- andSimpleExpr
-like "operator" methods.impl<T> ExprTrait for T where T: Into<SimpleExpr>
Expr
,SimpleExpr
, and also all otherInto<SimpleExpr>
types, likeValue
orFunctionCall
.impl PgExpr
forFunctionCall
,ColumnRef
,Keyword
,LikeExpr
,Value
.ExprTrait
: adding "operator" methods to all "smaller" types.impl SqliteExpr
forFunctionCall
,ColumnRef
,Keyword
,LikeExpr
,Value
.ExprTrait
: adding "operator" methods to all "smaller" types.impl From<LikeExpr> for SimpleExpr
fn like_like
. But this part is optional and I don't mind reverting this if you want.Bug Fixes
None
Breaking Changes
None
Changes
None
Unedited original notes
This is a "draft" PR with only implementation and almost no new documentation or tests. I'd like to hear some feedback before commiting to doing that.
I added a very minimal doctest on
ExprTrait
definition, as a showcase of expressions which are now possible to write without wrapping everything in layers ofExpr::something(...)
. If it's not impressive or hard to understand, please let me know, I'll add more examples/comparisons. See also examples in #771 and #770. It's hard to understate: THIS IS A HUGE USABILITY WIN. I use SeaORM at work, I'm a relatively experienced user (and a contributor) at this point, but theExpr
/SimpleExpr
stuff still drives me crazy from time to time.Apart from that doctest, there are no new tests, but note that
ExprTrait
methods are actually 100% covered by existing doctests of inherentExpr
/SimpleExpr
methods, which now call the trait under the hood.This PR proposes a small breaking change, and I plan a few more, described in "PR Info". Let's also discuss this. Are you open to merging these? I'd really like to see them included. It makes things SO ELEGANT, compared to what we had. I think, now is a great time for this, as you go through unstable release candidates before major SeaORM 1.0.0.