Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Libxlsxwriter API refactoring in progress - Porters take note. #252

Closed
jmcnamara opened this issue Nov 11, 2019 · 10 comments
Closed

Libxlsxwriter API refactoring in progress - Porters take note. #252

jmcnamara opened this issue Nov 11, 2019 · 10 comments

Comments

@jmcnamara
Copy link
Owner

jmcnamara commented Nov 11, 2019

Libxlsxwriter uses two kinds of structs as API parameters:

  1. Complex object like structs such as lxw_worksheet which are instantiated through the library and which are memory managed and freed by the library.
  2. Simple user instantiated structs such as lxw_image_options which are used for passing multiple options to a function.

The second type are always copied by the library and freed afterwards, i.e., the library never touches the public user data. However, some of these struct types are also used internally and some of the fields/members for metadata are shared with the external structs. This can cause issues where these fields aren't initialised by the user or just cause general confusion when inspected by the user.

This also make the API/ABI quite fragile since additional fields are often added to internal structs.

For the next version (0.8.8) of the library I'm going to rework the APIs so that external facing structs only contain documented fields and so that different structs are used internally if they require any additional metadata.

If you are porting or wrapping libxlsxwriter then this change will probably cause you some rework as well (although if you are just following the external documented APIs you will probably be okay).

Hopefully this will be a one-off overhead.

You can try out the changes from this rework on the api_refactor branch.

@jmcnamara jmcnamara changed the title API refactoring in progress - Porters take note. Libxlsxwriter API refactoring in progress - Porters take note. Nov 11, 2019
jmcnamara added a commit that referenced this issue Nov 11, 2019
jmcnamara added a commit that referenced this issue Nov 11, 2019
@jmcnamara
Copy link
Owner Author

jmcnamara commented Nov 12, 2019

@Alexhuszagh @RaFaeL-NN @sjmulder @Paxa@ @FTrautwein @gekola @HalfWayMan @Rolltrax @ropensci @yoeljacobsen @viest @fterrag

Please note this refactoring to libxlsxwriter which may affect your ports/wrappers.

jmcnamara added a commit that referenced this issue Nov 16, 2019
Refactored the chart fonts struct to document or remove hidden
fields.

See #252
jmcnamara added a commit that referenced this issue Nov 17, 2019
Documented optional file creation datetime in lxw_doc_properties.

See #252
jmcnamara added a commit that referenced this issue Nov 17, 2019
Made the deprecation of new_workbook() funtion more explicit
and removed instances from the examples and test code. The
workbook_new() function should be used instead.

See #252
jmcnamara added a commit that referenced this issue Nov 17, 2019
Refactored the data validation struct to remove hidden fields.

See #252
jmcnamara added a commit that referenced this issue Nov 17, 2019
Refactored the worksheet protection struct to remove hidden fields.

See #252
jmcnamara added a commit that referenced this issue Nov 17, 2019
Refactored the chart pattern struct to remove hidden fields.

See #252
@jmcnamara
Copy link
Owner Author

I've made a series of changes to fix the internal/external struct issues and document any unused (for now) fields. Hopefully this will make the ABI/API more stable in the future and less brittle for people instantiating structs to pass to the API.

I merge this up to master later and will release it in approximately the next day.

If anyone sees any issues let me know.

@RaFaeL-NN
Copy link

I get "error: wrong number of arguments specified for `deprecated' attribute" under MinGW

@RaFaeL-NN
Copy link

RaFaeL-NN commented Nov 17, 2019

Also, code for copying of user_props->created to doc_props does not exists in workbook_set_properties. If it's documented now, I think it must be or not?

@jmcnamara
Copy link
Owner Author

@RaFaeL-NN

I get "error: wrong number of arguments specified for `deprecated' attribute" under MinGW

I don't see that in my MinGW environment. How are you compiling it?

Also, code for copying of user_props->created to doc_props does not exists in workbook_set_properties. If it's documented now, I think it must be or not?

It is documented in the lxw_doc_properties struct. I'll add it to workbook_set_properties() as well. http://libxlsxwriter.github.io/structlxw__doc__properties.html

@RaFaeL-NN
Copy link

RaFaeL-NN commented Nov 17, 2019

How are you compiling it?

I don't know how to answer on this. I type "make" in MSYS under Windows

@jmcnamara
Copy link
Owner Author

jmcnamara commented Nov 17, 2019

@RaFaeL-NN

I don't know how to answer on this. I type "make" in MSYS under Windows

I followed the steps shown in the libxlsxwriter user guide for installing on Windows using Mingw-w64 and MSYS2 and when I type make I don't see that, or any other issue:

I'll add it to workbook_set_properties() as well.

I've added an explanation of the created parameter to the workbook_set_properties() docs:

http://libxlsxwriter.github.io/workbook_8h.html#aa814fd7f8d2c3ce86a7aa5d5ed127000

@RaFaeL-NN
Copy link

RaFaeL-NN commented Nov 17, 2019

I've added an explanation of the created parameter to the workbook_set_properties() docs:

I still can't see any code for this in workbook_set_properties in workbook.c at master. How it works? Is there an example?

@jmcnamara
Copy link
Owner Author

I still can't see any code for this in workbook_set_properties in workbook.c at master.

Sorry. I understand now. I had omitted copying the user value. I've fixed it on master.

Here is a working example:

#include "xlsxwriter.h"

int main() {

    lxw_workbook  *workbook   = workbook_new("doc_properties.xlsx");
    lxw_worksheet *worksheet  = workbook_add_worksheet(workbook, NULL);

    struct tm tmp_tm;
    tmp_tm.tm_year  = 119;
    tmp_tm.tm_mon   = 3;
    tmp_tm.tm_mday  = 17;
    tmp_tm.tm_hour  = 19;
    tmp_tm.tm_min   = 45;
    tmp_tm.tm_sec   = 15;
    tmp_tm.tm_isdst = -1;

    /* Create a properties structure and set some of the fields. */
    lxw_doc_properties properties = {
        .title    = "This is an example spreadsheet",
        .subject  = "With document properties",
        .author   = "John McNamara",
        .manager  = "Dr. Heinz Doofenshmirtz",
        .company  = "of Wolves",
        .category = "Example spreadsheets",
        .keywords = "Sample, Example, Properties",
        .comments = "Created with libxlsxwriter",
        .status   = "Quo",
        .created  = timegm(&tmp_tm),
    };

    /* Set the properties in the workbook. */
    workbook_set_properties(workbook, &properties);

    /* Add some text to the file. */
    worksheet_set_column(worksheet, 0, 0, 50, NULL);
    worksheet_write_string(worksheet, 0, 0,
                           "Select 'Workbook Properties' to see properties." , NULL);

    workbook_close(workbook);

    return 0;
}

@jmcnamara
Copy link
Owner Author

Merged upstream in version 0.8.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants