search

Electron100 / butane

An ORM for Rust with a focus on simplicity and on writing Rust, not SQL
Apache License 2.0
91 stars 13 forks source link

WIP: Add #[derive(ButaneJson)] #49

Closed jayvdb closed 1 year ago

jayvdb commented 1 year ago

This makes it easy to mark a struct with #[butane_type(Json) and #[derive(ButaneJson)] , and then use the struct in a model.

There are likely to be much better ways of doing this.

Electron100 commented 1 year ago

Sorry for the slow reply here, but I'm not sure I'm wild about loading up the current migration to check whether there's been a butane_type association for for JSON. Since we're supporting only JSON, perhaps it would be more straightforward and foolproof to just automatically make that binding at the same time. Like this: https://github.com/Electron100/butane/commit/fefe26138ea6c0cf56085d50453bb2e3e94a9274

jayvdb commented 1 year ago

I anticipated this FieldType derive would be extended to work for enums, and other "custom" types that are support for popular third party libs. The restriction on only supporting Json is just to show the concept quickly.

Yes there is a dependency on butane_type, but that is mandatory at the moment. So the question is whether your intended replacement for butane_type will be able to support this as well?

jayvdb commented 1 year ago

But I am happy to go with your approach, if it means we get this merged soon. We can worry about expanding this derive later.

Electron100 commented 1 year ago

I anticipated this FieldType derive would be extended to work for enums, and other "custom" types

Agreed. I think it can be extended to enums without any additional syntax. If we end up with cases where it's ambiguous what representation should be used we can introduce helper-attributes to the derive to allow further specification.

So the question is whether your intended replacement for butane_type will be able to support this as well?

Yes. My hope is for butane_type to go away entirely if I succeed in no longer having two separate views of the db type <=> Rust type mapping (the runtime one with real type info and the proc-macro one based solely on the AST the macros see). I'll admit aside from a shortage of time I've been a bit stymied by how to still preserve a good Butane CLI experience that doesn't require custom work in the codebase to help it. Regardless of the specifics there, butane_type is a crutch -- the goal is to specify the type used in the ToSql/FromSql impls at the same time as the type used for building the migration. So if we have an opportunity to do that with the FieldType derive, I think we should take it.

I am happy to go with your approach, if it means we get this merged soon.

Yup. I'm always open to being convinced I'm wrong, but if you're on board I'll merge your change + my amendment.

jayvdb commented 1 year ago

if you're on board I'll merge your change + my amendment.

Sounds good. Please do that.

Electron100 commented 1 year ago

Merged in #68 as the CI here had failed.