Clean up the API and document everything
sethfowler opened this issue · 9 comments
This library should remain a thin layer over the C libclang
API, but there are definitely some cases where things can be made a bit easier to use, and everything needs to be documented. Examples:
- Some thought needs to be given to what should be exported by default in the top-level Clang module.
- It'd be good to go over the names of all the functions and make sure they're consistent. There are some functions that might be better off in a different module.
- If there are places where we can reduce the need to manually write out the ClangApp phantom type parameter, that'd be nice.
- Everything needs to be documented.
@ghorn, since you've been using the library recently, I'd love any feedback you have to offer about things that could be improved to make the library easier to use. Try out the llvm3.3
branch so you can see the state of current development.
The ClangApp monad has changed a bit. Note that your functions that previously had the type a -> ClangApp b
now need the type a -> ClangApp s b
. For example one function in my application has the type Cursor -> ClangApp s Identifier
. The s
is a phantom type parameter like that used for the ST
monad.
Hi Seth. I've been using safer-api
for a few days, and I've found it a dramatic improvement. I don't see llvm3.3
on your account, only on chetant's account. Should I checkout that one? I also see some other branches on your account which you might want to delete if they're fully merged.
The llvm3.3
branch here (chetant's account) is based on the safer-api
branch on my account. I'd recommend switching to the branch here.
I am supposed to build llvm3.3 from their svn trunk, right? There is no release or tagged svn version for 3.3?
I think there is a tagged version for 3.3, though don't quote me on that, but building it against trunk would be fine. (I actually use their git mirror.)
If it's an inconvenience don't worry about it. I hope to get libclang embedded sometime next week so these sorts of considerations become a thing of the past.
@chetant So in my review this is the primary release blocker, now that (hopefully) GHCi is working on both OS X and linux. (We also need to resolve whatever build issues you mentioned you were having on Ubuntu 13.04.)
My plans here are basically these:
- Separate the types declared by the FFI module into a different module, Clang.Internal.Types.
- Export those types, along with the current content of Clang.Monad and Clang.Traversal, from the root Clang module. Don't export them from the more specific modules like Clang.Cursor. This means that we can reuse names like
getSpelling
andgetType
in different modules (since modules like Clang.Cursor will be imported qualified) but users don't have to qualify the types as well, which doesn't offer much benefit. - Add a few utility functions where they might improve the user experience. I've written a
parseSourceFile
function that should be much easier to figure out for beginners than the combination of withIndex and withParse that you need now. That was an obvious gap to me. I haven't thought of any others at the moment, but maybe more will jump out. - Make sure that all functions are placed in the most logical module.
- Add Haddock docs for
Clang.hs
. - Add Haddock docs for
Comment.hs
. - Add Haddock docs for
Completion.hs
. - Add Haddock docs for
Cursor.hs
. - Add Haddock docs for
Debug.hs
. - Add Haddock docs for
Diagnostic.hs
. - Add Haddock docs for
File.hs
. - Add Haddock docs for
Module.hs
. - Add Haddock docs for
Remapping.hs
. - Add Haddock docs for
Source.hs
. - Add Haddock docs for
String.hs
. - Add Haddock docs for
Token.hs
. - Add Haddock docs for
TranslationUnit.hs
. - Add Haddock docs for
Type.hs
. - Add Haddock docs for
USR.hs
. - Add Haddock docs for
Version.hs
.
I think that should cover all my concerns, and from my perspective we'll be ready to make a release on Hackage once this is done.
Does this sound good to you? Are there any other problems we need to tackle?
That makes sense. Will tackle the build issues on Linux, then work with you
to cleanup and document stuff.
Thanks for updating the api and cleaning it up considerably Seth!
On Apr 15, 2014 5:16 PM, "Seth Fowler" notifications@github.com wrote:
@chetant https://github.com/chetant So in my review this is the primary
release blocker, now that (hopefully) GHCi is working on both OS X and
linux. (We also need to resolve whatever build issues you mentioned you
were having on Ubuntu 13.04.)My plans here are basically these:
[ ] Separate the types declared by the FFI module into a different
module, Clang.Internal.Types.
[ ] Export those types, along with the current content of Clang.Monad and
Clang.Traversal, from the root Clang module. Don't export them from the
more specific modules like Clang.Cursor. This means that we can reuse names
like getSpelling and getType in different modules (since modules like
Clang.Cursor will be imported qualified) but users don't have to qualify
the types as well, which doesn't offer much benefit.
[ ] Add a few utility functions where they might improve the user
experience. I've written a parseSourceFile function that should be much
easier to figure out for beginners than the combination of withIndex and
withParse that you need now. That was an obvious gap to me. I haven't
thought of any others at the moment, but maybe more will jump out.
[ ] Make sure that all functions are placed in the most logical module.
[ ] Add Haddock docs for every type and function. Most of them can be
brought over from the underlying libclang's Index.h, but they'll
naturally require some cleanup.I think that should cover all my concerns, and from my perspective we'll
be ready to make a release on Hackage once this is done.Does this sound good to you? Are there any other problems we need to
tackle?—
Reply to this email directly or view it on GitHubhttps://github.com//issues/23#issuecomment-40541357
.
I'm glad I could help!
I checked off the first point about Clang.Internal.Types, but though I experimented with that approach, I decided against it. It turned out to be much more painful than I expected - Greencard isn't really set up to make this sort of split nice. It may be worth revisiting in the future, but for now I just changed where the types were exported without actually moving them to a new module.
Now that I've done that, though, I'm not sure it was the best move for every type. Many of the enumerations, in particular, seem like they might make more sense in a specific module after all. I do think this continues to look like the right choice for core types (TranslationUnit, Cursor, Type, etc.), though.