MAXakaWIZARD / xls-writer

Port of PEAR Spreadsheet Excel Writer
3 stars 2 forks source link

equations spelled with lower case cause exceptions #3

Closed themolecule closed 9 years ago

themolecule commented 9 years ago

This may generate an invalid xls equation, but shouldn't necessarily cause an exception:

It would seem that if the expression is not spelled as uppercase SUM, the parser fails to recognize it and throws an exception.

<?

require( dirname(__FILE__)."/include/xls-writer-master/vendor/autoload.php" );

use Xls\Workbook;

$workbook = new Xls\Workbook();

$s1 =& $workbook->addWorksheet("S1");
$s2 =& $workbook->addWorksheet("S2");

$f = $workbook->addFormat( array("numFormat"=>"$0.00") );

$s1->write(2,2,"=S2!C3",$f);
$s1->write(2,3,"12.34",null);
$s1->write(2,4,"=sum(C3:C4)",null);
$s2->write(2,2,"56.78",$f);

header("Content-type: application/vnd.ms-excel");
header("Content-Disposition: attachment; filename=\"test.xls\"");
header("Expires: 0");
header("Cache-Control: must-revalidate, post-check=0,pre-check=0");
header("Pragma: public");

$workbook->save('php://output');

?>
MAXakaWIZARD commented 9 years ago

@themolecule Are you sure that lowercase function name is valid?

MAXakaWIZARD commented 9 years ago

@themolecule I checked out, lowercase functions do not work, so I think this is valid behavior.

MAXakaWIZARD commented 9 years ago

@themolecule We can throw more descriptive exception messages, such as Lowercase function calls are invalid

MAXakaWIZARD commented 9 years ago

@themolecule Lowercase functions work now (they were transformed to uppercase in formula parser, but there was small error in recognizing such functions). So please check it out in latest version.

themolecule commented 9 years ago

Great... Thanks!

Lowercase functions are not valid, but easy enough to parse.

The exception that was being thrown was not helpful, which made debugging difficult. I was generating an xls file with > 1000 lines, so finding that tiny bug took me a while.

The implementation in pear supported lowercase, so porting code that used it had these latent bugs.

Thanks so much for your help!

MAXakaWIZARD commented 9 years ago

@themolecule Thank you too and feel free to report issues or contribute.