apiaryio / drafter

API Blueprint Parser (C++)
https://apiblueprint.org/
MIT License
301 stars 54 forks source link

(C API breaking) Move drafter options declarations to implementation #766

Closed tjanc closed 4 years ago

tjanc commented 4 years ago

Avoid exposing options structures to allow adding options in an additive, non-breaking process.

To add an option:

kylef commented 4 years ago

I'd perhaps leave the "DO NOT MERGE" label because once merged this will block other releases. We should perhaps have Protagonist/drafter.js changes ready to go.

klokane commented 4 years ago

What about linked list instead of adding new option to public API?

struct drafter_option {
   const char* name;
   option_type type; /* enum { number, string, boolean } */
   void* value;
   drafter_option* next;
};

Then add option in following way

drafter_option* create_options();
result add_option(drafter_option* option, const char* name, option_type type, void* value);
/* return OK or ERR_UNKNOWN_OPTION_NAME .... */
tjanc commented 4 years ago

@klokane I would rather have a stronger contract (vs a list of string-void*). If we add an option to drafter, it seems ok to me that we also extend the API with an appropriate setter; the goal of this PR is to avoid breaking the API in such a case. I also thought about uniting the two options structures into one, but differentiating between parsing and serialisation felt right in the end.

tjanc commented 4 years ago

This should be ready to merge now, what do you think @klokane

kylef commented 4 years ago

@tjanc we'll need to update the documentation in the README at https://github.com/apiaryio/drafter#parsing-a-blueprint-to-a-json-or-yaml-string to use these newer APIs too.

kylef commented 4 years ago

Upon release we should update apiblueprint.org's code example too: https://github.com/apiaryio/apiblueprint.org/blob/d461165b151992ec55683d7d92cef86c3ae10c60/source/developers.html.md#using-the-native-parser-interface-cc (https://apiblueprint.org/developers.html)