PHPOffice / PhpSpreadsheet

A pure PHP library for reading and writing spreadsheet files
https://phpspreadsheet.readthedocs.io
MIT License
13.35k stars 3.47k forks source link

SetAutoSize does not work consistently based on NumberFormat #1129

Closed EvilJordan closed 5 years ago

EvilJordan commented 5 years ago

This is:

- [X] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

setAutoSize sets column widths in a consistent manner, based on cell contents, not affected by meta NumberFormat data

What is the current behavior?

setAutoSize returns the incorrect cell width when a NumberFormat is used, and is not consistent in its incorrect-ness when a color is applied to the same NumberFormat. See code and screenshot.

What are the steps to reproduce?

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();

//this is exactly the same as the built-in constanct FORMAT_TEXT, just moved inline for the code demo
const FORMAT_TEXT1 = '@';
//this is exactly the same as the built-in constanct FORMAT_ACCOUNTING_USD, just moved inline for the code demo
const FORMAT_ACCOUNTING_USD1 = '_("$"* #,##0.00_);_("$"* \(#,##0.00\);_("$"* "-"??_);_(@_)';
//modified NumberFormat to include [Red] coloring
const FORMAT_ACCOUNTING_USD2 = '_("$"* #,##0.00_);[Red]_("$"* \(#,##0.00\);_("$"* "-"??_);_(@_)';

$sheet = $spreadsheet->getActiveSheet();
$sheet->setCellValue('A1', '-100'); //write demo value
$sheet->setCellValue('B1', '-100'); //write demo value
$sheet->setCellValue('C1', '-100'); //write demo value
$sheet->getStyle('A1')->getNumberFormat()->setFormatCode(FORMAT_TEXT1); //apply generic format
$sheet->getStyle('B1')->getNumberFormat()->setFormatCode(FORMAT_ACCOUNTING_USD1); //apply accounting format
$sheet->getStyle('C1')->getNumberFormat()->setFormatCode(FORMAT_ACCOUNTING_USD2); //apply accounting format with red coloring
$sheet->getColumnDimension('A')->setAutoSize(true); //setAutoSize
$sheet->getColumnDimension('B')->setAutoSize(true); //setAutoSize
$sheet->getColumnDimension('C')->setAutoSize(true); //setAutoSize

$writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($spreadsheet);
$writer->save("./color.xlsx");

Screen Shot 2019-08-06 at 3 35 01 PM

Which versions of PhpSpreadsheet and PHP are affected?

1.8.2 - worked fine in 1.4.0

EvilJordan commented 5 years ago

I've identified the partial source of the problem as how vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Style/NumberFormat.php handles interpreting the format and erroneously considering the ACCOUNTING format as a date.

Commenting out the conditional at line 667: if (preg_match('/(\[\$[A-Z]*-[0-9A-F]*\])*[hmsdy](?=(?:[^"]|"[^"]*")*$)/miu', $format, $matches)) { and letting the code continue without evaluating and changing the Accounting format to a date/time results in the following output: Screen Shot 2019-08-07 at 2 48 33 PM

Getting closer...

MarkBaker commented 5 years ago

I believe that accounting formats should now be handled correctly with the recent changes since 1.8.2 was released

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is still an issue for you, please try to help by debugging it further and sharing your results. Thank you for your contributions.