iliaal / php_excel

PHP Extension interface to the Excel writing/reading library
http://ilia.ws
534 stars 131 forks source link

Duplicate formats added to output #223

Closed jwestbrook closed 6 years ago

jwestbrook commented 6 years ago

I have already reported this same problem to libxl - but it looks like there is some problem where a specific format that has a border is being duplicated for every cell that is written with that format.

I'm not as familiar with php <=> C interfaces but I do see in the php_excel_write_cell method where the format is being added to the workbook every time a NULL or LONG value is written.

I don't know if that is what is causing it - but I have a styles.xml file in the XLSX output with 70000 formats, when I only added 11 in the php script.

I can provide the files (both php script and output) if you would like - but since there is some sensitive data in it I don't want to post on a public issue.

johmue commented 6 years ago

Can you please paste a simple example in this ticket. Normally you would reuse a format which by its own is specified by a specific ID - afaik.

g10-fred commented 6 years ago

I've had this issue, in my case it was because the format was defined and used each time a cell was written (simple code oversight), php_excel/LibXL defines a new excel format each time "new ExcelFormat" is called.

# Write a Cell
$dateFormat = new ExcelFormat($xlBook);
$dateFormat->numberFormat(ExcelFormat::NUMFORMAT_DATE);
$xlSheet->write($row, $column, $xlBook->packDate($date), $dateFormat);

As @johmue said, I needed to re-use the formats , so I moved my formats to a class property array and put the format declarations in the class constructor/init method. Then used the class property format when writing cells.

# Define an Excel Format (Constructor/Init)
$this->format['date'] = new ExcelFormat($xlBook); 
$this->format['date']->numberFormat(ExcelFormat::NUMFORMAT_DATE);

# Write a Cell
$xlSheet->write($row, $column, $xlBook->packDate($date), $this->format['date']);

Not sure if your issue is the same but it sounds very similar.

johmue commented 6 years ago

@DropdeadFredd exactly!

$book = new \ExcelBook(null, null, true);
$sheet = new \ExcelSheet($book, 'Table1');
$row = 1;
$col = 1;

// creates 10.000 individual formats in $book
for ($i=0; $i < 10000; $i++) {
    $format = new ExcelFormat($book);
    $format->numberFormat(ExcelFormat::NUMFORMAT_NUMBER);
    $sheet->write($row, $col, $i, $format);
}
$book->save('bad.xlsx');

while

$book = new \ExcelBook(null, null, true);
$sheet = new \ExcelSheet($book, 'Table1');
$row = 1;
$col = 1;

$format = new ExcelFormat($book);
$format->numberFormat(ExcelFormat::NUMFORMAT_NUMBER);

// uses 1 format in $book
for ($i=0; $i < 10000; $i++) {
    $sheet->write($row, $col, $i, $format);
}
$book->save('good.xlsx');

You can spot the difference already in the filesize of the output. Sometime Excel is able to remove the redundant formats when you open the file but it's not guaranteed.

jwestbrook commented 6 years ago

So here is a snippet of code from the script that builds the report.

I create all the formats at the top and then use the returned reference for the rest of the script.

    $workbook = new ExcelBook("LICENSE NAME", "LICENSE", true);

    $colheadingfont = $workbook->addFont();
    $colheadingfont->bold(true);
    $colheadingfont->name('arial');
    $colheadingfont->color(ExcelFormat::COLOR_WHITE);

    $currencyformat = $workbook->addCustomFormat("[$$-409]#,##0;-[$$-409]#,##0");
    $currency = $workbook->addFormat();
    $currency->numberFormat($currencyformat);
    $currency->horizontalAlign(ExcelFormat::ALIGNH_RIGHT);

    $date_time = $workbook->addFormat();
    $date_time->numberFormat(ExcelFormat::NUMFORMAT_CUSTOM_MDYYYY_HMM);

    $date_only = $workbook->addFormat();
    $date_only->numberFormat(ExcelFormat::NUMFORMAT_DATE);

    $center_data = $workbook->addFormat();
    $center_data->horizontalAlign(ExcelFormat::ALIGNH_CENTER);

    $percent = $workbook->addFormat();
    $percent->horizontalAlign(ExcelFormat::ALIGNH_CENTER);
    $percent->numberFormat(ExcelFormat::NUMFORMAT_PERCENT);

    $greyheader = $workbook->addFormat();
    $greyheader->horizontalAlign(ExcelFormat::ALIGNH_CENTER);
    $greyheader->setFont($colheadingfont);
    $greyheader->patternForegroundColor(ExcelFormat::COLOR_GRAY80);
    $greyheader->fillPattern(1);
    $greyheader->borderStyle(2);
    $greyheader->borderColor(ExcelFormat::COLOR_WHITE);

    $purpleheader = $workbook->addFormat();
    $purpleheader->setFont($colheadingfont);
    $purpleheader->patternForegroundColor(ExcelFormat::COLOR_DARKPURPLE_CF);
    $purpleheader->fillPattern(1);
    $purpleheader->borderBottomStyle(1);
    $purpleheader->borderColor(ExcelFormat::COLOR_PLUM);

    $purplecenterheader = $workbook->addFormat();
    $purplecenterheader->setFont($colheadingfont);
    $purplecenterheader->patternForegroundColor(ExcelFormat::COLOR_DARKPURPLE_CF);
    $purplecenterheader->fillPattern(1);
    $purplecenterheader->borderBottomStyle(1);
    $purplecenterheader->horizontalAlign(ExcelFormat::ALIGNH_CENTER);
    $purplecenterheader->borderColor(ExcelFormat::COLOR_PLUM);

    $GreyLightBold = $workbook->addFormat();
    $GreyLightBold->fillPattern(1);
    $GreyLightBold->patternForegroundColor(ExcelFormat::COLOR_GRAY25);
    $GreyLightBold->wrap(true);
    $GreyLightBold->verticalAlign(ExcelFormat::ALIGNV_TOP);

    $orderrow_format = $workbook->addFormat();
    $orderrow_format->fillPattern(1);
    $orderrow_format->patternForegroundColor(ExcelFormat::COLOR_GRAY25);

$orderdata = $workbook->addSheet('Completed Order Data');

$row = 2;

    foreach($orders as $o)
    {
        $colnum = 0;
        $orderdata->write($row,$colnum++,$o->id);
        $orderdata->write($row,$colnum++,$o->companyname);
        $orderdata->write($row,$colnum++,$o->loanreference);
        $orderdata->write($row,$colnum++,$o->propertyaddress);
        $orderdata->write($row,$colnum++,$o->propertystate);
    $orderdata->write($row,$colnum++,$workbook->packDate(strtotime($o->start_dts)),$date_only,ExcelFormat::AS_DATE);
 $row++;
}
g10-fred commented 6 years ago

@jwestbrook Sorry not to be any help but that looks OK to me, I can't see why you're getting the extra formats in excel.

jwestbrook commented 6 years ago

I think I found the problem

In the write cell method

https://github.com/iliaal/php_excel/blob/master/excel.c#L2405

if the value being written to the sheet is null or in my case sometimes the column from the data base is an empty string '' and there is no format being passed into the method (as in the example script above) then a NULL format is created every time the write method is called (instead of it being reused)

I believe thats what the ini setting prevents - but I can't set that as I need to apply colors/styles to other blank cells in other scripts (the clients like their pretty colors).

I opened a pull request https://github.com/iliaal/php_excel/pull/224 that removes the extra creation of the null/empty format

johmue commented 6 years ago

Nice spot! I would probably go along with "NULL" instead of "0" but that's maybe a personal taste thing. I still wonder why it is undocumented.

jwestbrook commented 6 years ago

PR #224 was merged - closing this issue