msoucy / dproto

D Protocol Buffer mixins to create structures at compile time
Boost Software License 1.0
37 stars 16 forks source link

questions #5

Closed timotheecour closed 10 years ago

timotheecour commented 10 years ago

I have a few questions.

enum s=message Foo {...};

//how to use that? ProtocolBuffer!s;

//for this: mixin(ProtocolBufferString!s) inspecting code produced with writeln shows stuff like: /// @todo: Figure out how to handle this stuff and other comments. so my question is: how ready is it? what's done and left to be be done?

I couldn't see any test examples?

do you handle Importing Definitions? (cf https://developers.google.com/protocol-buffers/docs/proto: importing definitions)

msoucy commented 10 years ago

Sorry about these issues - I'll be improving documentation on these shortly.

The intended way to use it is:

// From a string:
mixin ProtocolBuffer!s;
// From a file (does basic parsing)
mixin ProtocolBuffer!"somefile.proto";

The TODO is accurate - it does not properly handle ignoring unknown message parts

I believe importing definitions works properly.

timotheecour commented 10 years ago

ok thanks, maybe just a unittest would serve as doc for now. I think it'd be better not to overload ProtocolBuffer to work on both files and strings; I'd rather have: ProtocolBufferFromFile and ProtocolBufferFromString (or whatever so long these are different names)

For importing definitions, could you add a search path (eg equivalent of -I flag)

do you have a list not-yet-supported protocol buffer features?

msoucy commented 10 years ago

Search paths are added using -J because they're string imports

I'd like the most common case to be the most straightforward - to me, this is parsing from a file instead of from a literal. So, how do the following sound:

ProtocolBuffer!filename;
ProtocolBufferFromString!str;
// with "Raw" versions for generating strings only
msoucy commented 10 years ago

I'll try to create a list of unsupported features tonight, but for now I can say that it does not support the deprecated groups, nor will it.

timotheecour commented 10 years ago

So, how do the following sound:

sounds good. Problems always arise with conflation (eg: if one wants to have a file with non-standard extension, that would fail with old scheme)

timotheecour commented 10 years ago

if dproto is the user facing API, aren't ProtocolBuffer and ProtocolBufferString redundant? can ProtocolBufferString be hidden somewhere else so user would only call: ProtocolBuffer!filename //the one just for files ProtocolBufferFromString!str //the one just for strings (since ProtocolBuffer just calls ProtocolBufferString now)

msoucy commented 10 years ago

After some recent changes (recent being a relative term), and separating out the .proto and string parsers, ProtocolBufferString is just the same thing as ParseProtoSchema(filename, data).toD()...as such, I agree that it's not really needed to have it anymore.

Also, made issues for the major things that I believe it's missing

msoucy commented 10 years ago

I'm closing this issue, as #6 #7 #8 #9 #10 #11 #12 have been split out. This will allow me to keep better track of what's needed.