Skip to content

Commit

Permalink
Handling of User-supplied Decimal and Thousands Separators
Browse files Browse the repository at this point in the history
Fix PHPOffice#3900. The code to adjust Decimal and Thousands separators to user-specified choices expects to convert periods to commas and vice versa - hard-coded without taking the user specification into account. This is likely to be the only significant use case, however, as the issue demonstrates, it is not the only possibility. In particular, it will mess up Csv output if decimal separator is kept as period, but thousands separator is set to null string. The code is altered to replace period with the user-selected decimal separator, and comma with the user-selected thousands separator. No existing tests broke as a result of this change in behavior.
  • Loading branch information
oleibman committed Feb 15, 2024
1 parent d620497 commit e1fb68e
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Style/NumberFormat/BaseFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ protected static function adjustSeparators(string $value): string
$thousandsSeparator = StringHelper::getThousandsSeparator();
$decimalSeparator = StringHelper::getDecimalSeparator();
if ($thousandsSeparator !== ',' || $decimalSeparator !== '.') {
$value = str_replace(['.', ',', "\u{fffd}"], ["\u{fffd}", '.', ','], $value);
$value = str_replace(['.', ',', "\u{fffd}"], ["\u{fffd}", $thousandsSeparator, $decimalSeparator], $value);
}

return $value;
Expand Down
22 changes: 22 additions & 0 deletions tests/PhpSpreadsheetTests/Shared/StringHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
namespace PhpOffice\PhpSpreadsheetTests\Shared;

use PhpOffice\PhpSpreadsheet\Shared\StringHelper;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Writer\Csv;
use PHPUnit\Framework\TestCase;

class StringHelperTest extends TestCase
Expand Down Expand Up @@ -98,4 +100,24 @@ public function testSYLKtoUTF8(): void

self::assertEquals($expectedResult, $result);
}

public function testIssue3900(): void
{
StringHelper::setDecimalSeparator('.');
StringHelper::setThousandsSeparator('');

$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setCellValue('A1', 1.4);
$sheet->setCellValue('B1', 1004.5);
$sheet->setCellValue('C1', 1000000.5);

ob_start();
$ioWriter = new Csv($spreadsheet);
$ioWriter->setDelimiter(';');
$ioWriter->save('php://output');
$output = ob_get_clean();
$spreadsheet->disconnectWorksheets();
self::assertSame('"1.4";"1004.5";"1000000.5"' . PHP_EOL, $output);
}
}

0 comments on commit e1fb68e

Please sign in to comment.