-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Ensure that range related functions had a valid range to work with #29
Conversation
@@ -600,6 +600,11 @@ public static function absoluteCoordinate($pCoordinateString = 'A1') | |||
*/ | |||
public static function splitRange($pRange = 'A1:A1') | |||
{ | |||
// Ensure $pRange is a valid range | |||
if(empty($pRange)){ | |||
$pRange = 'A1:A1'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it good idea to rewrite this part as const DEFAULT_RANGE = 'A1:A1' / self::DEFAULT_RANGE. Mb. all things should be replaced? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be. I will give it a try ;)
Thanks. I'd already realised that the changes I'd merged for autofilter expressions had broken saving xls/xlsx files without any autofilter defined - I've implemented the fixes locally and will be committing/merging back to develop later today |
I just saw that you updated the code for savi xls/xlsx files without any autofilter. However the changes made directly to Cell class to ensure the range is valid would help to avoid similar problems in the future. I think that should be merged in the develop branch too. |
Thanks lowfill - I will add those validations as well - but I'd already been working on the fix since it was first brought to my attention and wanted to get it in place as quickly as I could. I still need to rerun all the tests to ensure I haven't missed anything critical, but I'll be committing the more comprehensive fix later (or possibly tomorrow). I'd just had so many people telling me that things were broken here, on codeplex, via mail, via twitter, even to my company email, that I wanted to get that quick fix committed and merged. It's also why I've still left this open for the moment |
Yeap, that was an anoying issue. Yesterday, I spent the whole day debugging it. For sure all the changes that you are doing will be good for all |
Manual merge will be committed later this evening, thanks for all the work you've done on this |
I have same problem with reader! Message: Cell coordinate can not be zero-length string #0 C:\xampp\htdocs\wimax.shabdiznet.com\library\PHPExcel\Reader\Excel2007.php(935): PHPExcel_Cell::coordinateFromString('') The uploaded file is file.xlsx |
Sorry Forgot to say that I got latest version from Github and problem still exists |
When I tried to run the tests I have this error:
Fatal error: Uncaught exception 'PHPExcel_Exception' with message 'Cell coordinate can not be zero-lenght string'
Looking throuhgt the code I discover that range related functions on Cell class didn't ensure a valid range when the argument is empty.
I fix this and all the test are working again.