search

naqvis / crystal-vips

Crystal bindings for the libvips image processing library.
MIT License
39 stars 5 forks source link

Update enums with missing members and `@[Flags]` annotation #6

Closed Sija closed 7 months ago

Sija commented 7 months ago

Resolves #5 Refs https://github.com/libvips/libvips/pull/3370

Sija commented 7 months ago

While we're at it, shouldn't VipsOperationFlags and VipsForeignFlags enums have @[Flags] annotation too?

naqvis commented 7 months ago

While we're at it, shouldn't VipsOperationFlags and VipsForeignFlags enums have @[Flags] annotation too?

Thanks @Sija 👍 . Yeah agree these two enums should be flaged with @[Flags] annotation. Appreciate if you can please apply these changes along with this PR.

Sija commented 7 months ago

@naqvis Done.

Sija commented 7 months ago

I'm also wondering, are long enum member names intentional? VipsArgumentFlags::Required vs VipsOperationFlags::VipsOperationSequential

Sija commented 7 months ago

I'd suggest to drop the prefixes from the enum member names, like in VipsArgumentFlags.

naqvis commented 7 months ago

I'd suggest to drop the prefixes from the enum member names, like in VipsArgumentFlags.

Yeah agree. Reason they are there is because this lib.cr was generated via crystal_lib and I didn't spend time on fixing generated code. Definitely would be great if such repetition can be avoided.

naqvis commented 7 months ago

I'm going to go with merging this PR, while can leave the refactoring/clean-up task to separate PR.