Skip to content

Conversation

@deekthesqueak
Copy link
Contributor

@deekthesqueak deekthesqueak commented Jun 21, 2018

This is:

- [x] a bugfix

Checklist:

Why this change is needed?

This custom formatting commonly shows up in exported timesheet data from a large company.

When the escaped space isn't kept it will lead this to the format array being exploded into 3 pieces. Then at https://github.com/PHPOffice/PhpSpreadsheet/blob/1.3.1/src/PhpSpreadsheet/Style/NumberFormat.php#L468 the check only only take the firs block which is 'hh:mm:ss' so it assumes 24 hour but then later in the loop you'll have 'am/pm' which gets converted into A. When all is said and done you can get AM/PM on a 24 hour value.

@deekthesqueak deekthesqueak changed the title Fixes issue where escaped spaces in date format would cause incorrect… Fix #557: Handle escaped spaces in date format would cause incorrect AM/PM conversion Jun 21, 2018
date format leading to incorrect time conversions
Copy link
Member

@PowerKiKi PowerKiKi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Could you please update the CHANGELOG as well in the unreleased section ?

@PowerKiKi PowerKiKi merged commit 7a4cbd4 into PHPOffice:develop Jun 24, 2018
@PowerKiKi
Copy link
Member

Thanks !

@deekthesqueak deekthesqueak deleted the Bug557 branch June 24, 2018 21:26
Dfred pushed a commit to Dfred/PhpSpreadsheet that referenced this pull request Nov 20, 2018
Fix issue where escaped spaces in cell format would cause incorrect
date format leading to incorrect time conversions

Fixes PHPOffice#557
Closes PHPOffice#558
BlackyTay pushed a commit to BlackyTay/PhpSpreadsheet that referenced this pull request Aug 8, 2025
Fix issue where escaped spaces in cell format would cause incorrect
date format leading to incorrect time conversions

Fixes PHPOffice#557
Closes PHPOffice#558
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants