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

Clean loading #30

Merged
merged 12 commits into from
Jan 10, 2022
Merged

Clean loading #30

merged 12 commits into from
Jan 10, 2022

Conversation

JanMarvin
Copy link
Owner

@JanMarvin JanMarvin commented Jan 10, 2022

remove a few old unused functions and clean up the loading code a bit more

* cppReadFile
* removeHeadTag
* readUTF8
…hey are all straight in a single line like this <a><b><c foo="bar/><b/><c/>.
* getAttr
* build_table_xml: writeDataTable is still broken, but this should replace the function
* itos
* markUTF8
* don't overwrite existing celXfs with writeData2 (well well, if it isn't my old enemy: me)
@JanMarvin
Copy link
Owner Author

JanMarvin commented Jan 10, 2022

Codecov Report

Merging #30 (c05d70d) into main (214d32a) will increase coverage by 0.22%.
The diff coverage is 77.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   35.49%   35.71%   +0.22%     
==========================================
  Files          45       45              
  Lines       10566    10531      -35     
==========================================
+ Hits         3750     3761      +11     
+ Misses       6816     6770      -46     
Impacted Files Coverage Δ
R/dates.R 46.15% <0.00%> (ø)
R/helperFunctions.R 14.97% <ø> (-1.61%) ⬇️
R/readWorkbook.R 38.29% <0.00%> (+0.79%) ⬆️
src/load_workbook.cpp 74.37% <ø> (+6.16%) ⬆️
src/write_file.cpp 73.97% <ø> (-3.68%) ⬇️
src/getXML.cpp 11.49% <50.00%> (+0.65%) ⬆️
R/loadWorkbook.R 67.58% <60.00%> (+0.33%) ⬆️
src/pugi.cpp 41.29% <79.16%> (+4.53%) ⬆️
R/class-workbook.R 26.59% <90.32%> (+0.07%) ⬆️
R/pugixml.R 49.25% <100.00%> (+2.98%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 214d32a...c05d70d. Read the comment docs.


.self$tables <- xml_node_create(xml_name = "table",
xml_children = c(autofilter, tableColumn, tableStyleXML),
xml_attributes = table_attrs)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might not look like much, but this is why I'm doing this. Removing weird functions dangling with code in unpredictable manners and replacing them with simple functions that do the heavy lifting behind the scenes.
If one wants a new attribute, just add it to the xml_attributes table. No need to dangle with hundreds of lines of barely readable Rcpp code. And I say that with the greatest respect. Someone has written all that old code and one day someone might look at my code and is like "what the heck is this?" Here we are, taking baby steps everyone :-)

@JanMarvin JanMarvin merged commit 2d73d94 into main Jan 10, 2022
@JanMarvin JanMarvin deleted the clean_loading branch January 10, 2022 23:30
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

Successfully merging this pull request may close these issues.

None yet

1 participant