Open pachadotdev opened 5 years ago
You'll have to also change the full.names and include.dirs options to zipr to TRUE. help(zip::zipr) says...
For ‘zipr’ (and ‘zipr_append’), each specified file or directory in files is created as a top-level entry in the zip archive.
while...
For ‘zip’ (and ‘zip_append’), the root of the archive is supposed to be the current working directory. The paths of the files are fully kept in the archive. Absolute paths are also kept.
so line 562 in WorkbookClass.R should be zip::zipr(zipfile = tmpFile, files = list.files(tmpDir, full.names = TRUE, recursive = TRUE, include.dirs = TRUE, all.files = TRUE), compression_level = cl)
otherwise it comes out a mess
Furthermore, it affects reading the file after it's saved... Yeah, this pull request ain't working out for me on Ubuntu 18.04.1, LibreOffice Calc, and
platform x86_64-pc-linux-gnu
version.string R version 3.5.3 (2019-03-11) nickname Great Truth
hi @mgavin, I'm using this modification in a server (Ubuntu Server 18.04) to export the output to xlsx, and it show no problem to read the downloaded file in LibreOffice and Microsoft Excel. I'll dig the problem a bit more
I think it was a multitude of things. Like, when looking at the underlying xml that gets zipped up into an xlsx, my sheet1.xml had a tag with an attribute like "maxcols = 1025", and then openxlsx would turn that into 1025 column tags... So I'm thinking things got lost in translation, and zipping the xml back up turned it into something unreadable.
But I think the list.files options should be changed, because zip kept the relative pathing, while zipr makes every file/directory a top level entry.
from help(zip::zip)
The different between 'zipr' and 'zip' is how they handle the relative paths of the input files.
to test: (without zipr)
> library(openxlsx)
> wb <- createWorkbook()
> saveWorkbook(wb, "sampleWorkbook", overwrite = TRUE)
Warning: Workbook does not contain any worksheets. A worksheet will be added.
Note: zip::zip() is deprecated, please use zip::zipr() instead
> zip::zip_list("sampleWorkbook")$filename
[1] "[Content_Types].xml"
[2] "_rels/.rels"
[3] "docProps/app.xml"
[4] "docProps/core.xml"
[5] "xl/_rels/workbook.xml.rels"
[6] "xl/printerSettings/printerSettings1.bin"
[7] "xl/styles.xml"
[8] "xl/theme/theme1.xml"
[9] "xl/workbook.xml"
[10] "xl/worksheets/_rels/sheet1.xml.rels"
[11] "xl/worksheets/sheet1.xml"
(with zipr, as in the commit)
> library(openxlsx)
> wb <- createWorkbook()
> saveWorkbook(wb, "sampleWorkbook2")
Warning: Workbook does not contain any worksheets. A worksheet will be added.
> zip::zip_list("sampleWorkbook2")$filename
[1] "[Content_Types].xml" ".rels" "app.xml"
[4] "core.xml" "workbook.xml.rels" "printerSettings1.bin"
[7] "styles.xml" "theme1.xml" "workbook.xml"
[10] "sheet1.xml.rels" "sheet1.xml"
(and with zipr, with the changes to the options)
Actually, while I was testing this, I got a different list of stuff, even from the original.
SO, to get the original list of files (to how zip::zip put in the files), just make the line
zip::zipr(zipfile = tmpFile, files = list.files(tmpDir, full.names = FALSE, recursive = FALSE, include.dirs = FALSE, all.files=TRUE, no.. = TRUE), compression_level = cl)
changing recursive to FALSE, and no.. to TRUE
> library(openxlsx); wb <- createWorkbook(); saveWorkbook(wb, "sampleWorkbook11"); zip::zip_list("sampleWorkbook11")$filename
Warning: Workbook does not contain any worksheets. A worksheet will be added.
[1] "[Content_Types].xml"
[2] "_rels/"
[3] "_rels/.rels"
[4] "docProps/"
[5] "docProps/app.xml"
[6] "docProps/core.xml"
[7] "xl/"
[8] "xl/_rels/"
[9] "xl/_rels/workbook.xml.rels"
[10] "xl/drawings/"
[11] "xl/drawings/_rels/"
[12] "xl/printerSettings/"
[13] "xl/printerSettings/printerSettings1.bin"
[14] "xl/styles.xml"
[15] "xl/tables/"
[16] "xl/tables/_rels/"
[17] "xl/theme/"
[18] "xl/theme/theme1.xml"
[19] "xl/workbook.xml"
[20] "xl/worksheets/"
[21] "xl/worksheets/_rels/"
[22] "xl/worksheets/_rels/sheet1.xml.rels"
[23] "xl/worksheets/sheet1.xml"
Okay, the lists aren't exactly the same... but the paths are there. Conclusion: The options should be changed
Codecov Report
59.42% <55.55%> (ø)
Continue to review full report at Codecov.