8000 [VarDumper] Improve microseconds support in date caster by maidmaid · Pull Request #24030 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[VarDumper] Improve microseconds support in date caster #24030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 4, 2017
Merged

[VarDumper] Improve microseconds support in date caster #24030

merged 1 commit into from
Sep 4, 2017

Conversation

maidmaid
Copy link
Contributor
@maidmaid maidmaid commented Aug 29, 2017 •
edited
Loading
Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets /
License MIT
Doc PR /

This PR adds the microseconds in period virtual property and in tooltip overview.

Before:

After:

And fixes interval caster:

dump((new DateTime('00:00:00'))->diff(new DateTime('00:00:00.123')));

// before
DateInterval {
  interval: + 00:00:00.0.123
}

// after
DateInterval {
  interval: + 00:00:00.123
}

@maidmaid maidmaid force-pushed the vardumper-period-ms branch from 9ad9cbe to 98bb271 Compare August 29, 2017 21:51
@maidmaid
Copy link
Contributor Author

Fail not related.

@nicolas-grekas
Copy link
Member

can you add a test with non-zero ms?

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Aug 31, 2017
@maidmaid
Copy link
Contributor Author

Mmh, non-zero ms cases are based on the formatSeconds() method which is already tested here:

symfony/src/Symfony/Component/VarDumper/Tests/Caster/DateCasterTest.php

Lines 91 to 96 in 89d85f3

array('2017-04-30 00:00:00.100000', '+00:00', '2017-04-30 00:00:00.100 +00:00', 1493510400, 'Sunday, April 30, 2017%Afrom now'),
array('2017-04-30 00:00:00.120000', '+00:00', '2017-04-30 00:00:00.120 +00:00', 1493510400, 'Sunday, April 30, 2017%Afrom now'< 8000 /span>),
array('2017-04-30 00:00:00.123000', '+00:00', '2017-04-30 00:00:00.123 +00:00', 1493510400, 'Sunday, April 30, 2017%Afrom now'),
array('2017-04-30 00:00:00.123400', '+00:00', '2017-04-30 00:00:00.123400 +00:00', 1493510400, 'Sunday, April 30, 2017%Afrom now'),
array('2017-04-30 00:00:00.123450', '+00:00', '2017-04-30 00:00:00.123450 +00:00', 1493510400, 'Sunday, April 30, 2017%Afrom now'),
array('2017-04-30 00:00:00.123456', '+00:00', '2017-04-30 00:00:00.123456 +00:00', 1493510400, 'Sunday, April 30, 2017%Afrom now'),
Is it really usefull?

@nicolas-grekas nicolas-grekas mentioned this pull request Aug 31, 2017
Closed
@maidmaid maidmaid force-pushed the vardumper-period-ms branch from febfdba to cbbd3b2 Compare August 31, 2017 17:18
@maidmaid maidmaid changed the title [VarDumper] Add more precision in period caster [VarDumper] Improve microseconds support in date caster Aug 31, 2017
@nicolas-grekas
Copy link
Member

tests are red

@maidmaid maidmaid force-pushed the vardumper-period-ms branch from cbbd3b2 to 5c37b76 Compare September 4, 2017 13:49
@maidmaid maidmaid force-pushed the vardumper-period-ms branch from 5c37b76 to 479b5d6 Compare September 4, 2017 16:34
@maidmaid
Copy link
Contributor Author
maidmaid commented Sep 4, 2017

Fixed.

@nicolas-grekas
Copy link
Member

Thank you @maidmaid.

@nicolas-grekas nicolas-grekas merged commit 479b5d6 into symfony:3.4 Sep 4, 2017
nicolas-grekas added a commit that referenced this pull request Sep 4, 2017
@nicolas-grekas
bug #24030 [VarDumper] Improve microseconds support in date caster (m…
aad90fa 
…aidmaid)

This PR was merged into the 3.4 branch.

Discussion
----------

[VarDumper] Improve microseconds support in date caster

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | /
| License       | MIT
| Doc PR        | /

This PR adds the microseconds in ``period`` virtual property and in tooltip overview.

Before:
![](https://user-images.githubusercontent.com/4578773/29843773-58a2e212-8d0d-11e7-9053-3eb1cf933e3b.png)

After:
![](https://user-images.githubusercontent.com/4578773/29843774-58bd8342-8d0d-11e7-8d83-40835c89313c.png)

And fixes interval caster:

```php
dump((new DateTime('00:00:00'))->diff(new DateTime('00:00:00.123')));

// before
DateInterval {
  interval: + 00:00:00.0.123
}

// after
DateInterval {
  interval: + 00:00:00.123
}
```

Commits
-------

479b5d6 Improve microseconds support in date caster
@maidmaid maidmaid deleted the vardumper-period-ms branch September 4, 2017 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature Status: Needs Review VarDumper
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
3 participants
@maidmaid @nicolas-grekas @carsonbot
0