search

orbitz / ocaml-protobuf

Protobuf implementation in Ocaml
BSD 3-Clause "New" or "Revised" License
7 stars 1 forks source link

submodules visible but not linked #4

Open agarwal opened 11 years ago

agarwal commented 11 years ago

Modules are packed into the Protobuf module. However, it seems the sub-modules are visible though unusable. For example, after doing #require "protobuf";;, utop will autocomplete to Builder and other sub-modules. However, it'll give the error Error: Reference to undefined globalBuilder' `. I don't use OCaml's pack option, so am not sure exactly what the error is.

orbitz commented 11 years ago

Hrm, based on your other post it looks like you can use Builder, though? But utop has no problems with Core right?

agarwal commented 11 years ago

Yes, I can use the library fine by referring to the intended module path Protobuf.Builder. However, at first, I tried directly using Builder (without opening Protobuf). Since utop auto-completes it, it is somehow in scope.

In any case, I would recommend manually packing. OCaml's builtin pack option is too restrictive. For example, in Biocaml, we name every module Biocaml_foo. Then in biocaml.ml, we manually type module Foo = Biocaml_foo.

orbitz commented 11 years ago

I've gone back and forth on packing myself or not. I think packs are really easy to use but seems like they are a huge hack. I have a feeling this issue has to do with the .mli files being installed. I'll probably try to move to manual packs like biocaml, at least to see how well it works.

agarwal commented 10 years ago

This might be something with utop as I've seen it in other libraries. In any case, how about going with manual packing? At the same time, I'd recommend putting a Protobuf_ prefix on all modules. If you're okay with that, I'll submit a pull request.

orbitz commented 10 years ago

Is this a known bug in utop? For the most part I like packing as if the library is small and removing packing is low priority.

agarwal commented 10 years ago

Is this a known bug in utop?

Not sure. As you say it could be due to mli files being installed. Maybe utop picks that up. This doesn't actually affect anything, so let's just leave it for now.

orbitz commented 10 years ago

Hrm, well, after thinking about it for a day, doesn't seem unreasonable to try self packing. If you can rebase your change and PR I can merge it if it looks good. Thanks!

agarwal commented 10 years ago

Nothing to rebase as I hadn't done this yet. I'll submit a pull request.