add ability to set codepage explicitly for BIFF5 #1484
Conversation
src/PhpSpreadsheet/Reader/Xls.php
Outdated
| /** | ||
| * @param string $codepage | ||
| */ | ||
| public function setCodepage($codepage): void |
There was a problem hiding this comment.
You might want to consider typehinting, and some form of validation here
There was a problem hiding this comment.
You might want to consider typehinting, and some form of validation here
done
src/PhpSpreadsheet/Reader/Xls.php
Outdated
| @@ -640,7 +648,7 @@ public function load($pFilename) | |||
|
|
|||
| // initialize | |||
| $this->pos = 0; | |||
| $this->codepage = 'CP1252'; | |||
| $this->codepage = $this->codepage == null ? 'CP1252' : $this->codepage; | |||
There was a problem hiding this comment.
Perhaps use $this->codepage = ?: 'CP1252' here.
As our minimum PHP version is 7.1, using more succinct syntax is available to us
|
Also, take a look at the most recent merges to master, one of which addresses codepages |
boltunoreh
force-pushed
the
master
branch
2 times, most recently
from
b1b7d18 to
80f4666
Compare
done |
| public static function validate(string $codePage): bool | ||
| { | ||
| if (!in_array($codePage, self::$pageArray)) { | ||
| return false; |
There was a problem hiding this comment.
Can be much simplified to a single line as return (in_array($codePage, self::$pageArray, true)); with strict type checking for the codepage string as well
If BIFF5 excel 95 file doesn't have codepage record, the default codepage CP1252 is used and can't be change. That causes to problems with decoding cyrillic text.
If BIFF5 excel 95 file doesn't have codepage record, the default codepage CP1252 is used and can't be change.
That causes to problems with decoding cyrillic text.
This is:
Checklist:
Why this change is needed?
It's made to add ability to read Excel 95 files with cyrillic text properly.