IreneKnapp / direct-sqlite

MIT License
35 stars 54 forks source link

Split FFI bindings into a separate package #10

Closed joeyadams closed 12 years ago

joeyadams commented 12 years ago

I made a new package for direct-sqlite's FFI bindings and enumeration types, but haven't released it to Hackage yet. I'm waiting for a response to this pull request first.

https://github.com/joeyadams/direct-sqlite-bindings

This means direct-sqlite can focus on value marshaling and error handling, without having to worry about foreign imports and enumerations. It also means persistent-sqlite and direct-sqlite can share the same copy of sqlite3.c without breaking any existing APIs.

This pull request updates direct-sqlite to use direct-sqlite-bindings. It does not change direct-sqlite's API, except for a couple bug fixes:

In direct-sqlite-bindings, I use newtype wrappers to distinguish among the various integer types in the API. For example:

foreign import ccall "sqlite3_bind_blob"
    c_sqlite3_bind_blob
        :: Ptr CStatement
        -> CParamIndex      -- ^ Index of the SQL parameter to be set
        -> Ptr a            -- ^ Value to bind to the parameter.
        -> CNumBytes        -- ^ Length, in bytes.  This must not be negative.
        -> Ptr CDestructor
        -> IO CError

This makes the function signatures much easier to understand. It'd be nice to have similar newtypes in direct-sqlite, but I wanted to avoid breaking the existing API for now.

nurpax commented 12 years ago

Just a thought, did you consider instead of separating the Bindings stuff into its own module but keeping the code in direct-sqlite? Direct-sqlite could switch to use the new C functions while persistent could talk to the Database/SQLite3/Bindings interface but no new package would be introduced.

joeyadams commented 12 years ago

Good idea, @nurpax. On the one hand, this means people who want to use the bindings directly, rather than Direct.SQLite3, will have to install a module they won't need. On the other hand, if they have a problem using Direct.SQLite3, then perhaps Direct.SQLite3 should be fixed instead.

I'm closing this pull request. I plan to make a new pull request that adds the modules Direct.SQLite3.Bindings and Direct.SQLite3.Bindings.Types to direct-sqlite, without having in the commit history that we removed sqlite3.c and then added it back again.