WIP: Fix Get-Date -UFormat `%V` week number output by timparkinson · Pull Request #4508 · PowerShell/PowerShell

Tim Parkinson (timparkinson) · 2017-08-05T10:07:45Z

PR Summary

Fix #4264 makes week number output compliant with ISO-8601.

PR Checklist

Ilya (iSazonov)

Tim Parkinson (@timparkinson) Thanks for your contribution!

Could you please review all uformat patterns - it seems we should fix all.

Ilya (iSazonov) · 2017-08-05T19:25:12Z

I believe we shouldn't create new object.
System.Threading.Thread.CurrentThread.CurrentCulture.Calendar

I don't believe using the CurrentCulture is appropriate in this case, because ISO 8601 is Gregorian calendar based and CurrentCulture could be set to a different calendar.

In any case further tests based on the table at https://en.wikipedia.org/wiki/ISO_week_date#Relation_with_the_Gregorian_calendar shows that my initial stab at this was incorrect, because the GetWeekOfYear does not produce the ISO 8601 output for all cases. I'll take a bit of a harder stare at this.

I believe we should follow CoreFX and should be CurrentCulture based not ISO based for all culture.
/cc Steve Lee (@SteveL-MSFT)

Agree with Ilya (@iSazonov) that in general, we should be using CurrentCulture. Not an expert by any means on this topic, but looking at the Wikipedia page, it seems we should implement the formula they have there?

CoreCLR already implemented this in GregorianCalendar(...)
We can use the implementation.

Ilya (iSazonov) · 2017-08-05T19:30:36Z

The tests may fail if current locale is not English. So we shouldn't use explicit constants - please evaluate them.

$a=Get-Culture $a.Calendar ...

Ilya (iSazonov) · 2017-08-18T16:45:06Z

Tim Parkinson (@timparkinson) Could you please continue?

Tim Parkinson (timparkinson) · 2017-08-18T17:28:28Z

Ilya (@iSazonov) yes, I'm happy to continue looking at this, but I haven't had time over the last week. I may be able to spend a bit of time this weekend.

I did start looking at the other UFormats and there are definitely others that don't match the output from the Linux date command.

Would you prefer me to address them as one PR or create separate issues for each?

Ilya (iSazonov) · 2017-08-21T17:39:41Z

If you are limited in time you can split the work on some PRs - describe all points in one Issue and fix step by step.

Tim Parkinson (timparkinson) · 2017-08-22T04:53:06Z

Ilya (@iSazonov) OK, I'll raise another issue detailing all the deviations from Linux behaviour in UFormat and then I'll look to fix step by step as you suggest.

I have a mini vacation after today, and will be without computer/internet - I'll hopefully be able to get to it next week.

Ilya (iSazonov) · 2017-08-22T05:11:58Z

/cc Steve Lee (@SteveL-MSFT) Changing UFormat looks like a breaking change - should we have the PowerShell committee's approvement?

Joey Aiello (joeyaiello) · 2017-08-25T19:22:40Z

Backchanneled with Steve Lee (@SteveL-MSFT), I'm good with these changes.

Steve Lee (SteveL-MSFT) · 2017-08-25T23:03:24Z

@PowerShell/powershell-committee discussed this briefly and agree the change makes sense

Ilya (iSazonov) · 2017-09-04T07:41:37Z

Tim Parkinson (@timparkinson) Please rebase.

Tim Parkinson (timparkinson) · 2017-09-04T08:31:04Z

As previously discussed, I intend to raise another issue to document all the UFormat divergences since there are several. Since this PR is incorrect anyway, I guess I should simply close it?

Ilya (iSazonov) · 2017-09-04T10:41:53Z

It would be right to open the new Issue first and then decide on the fate of that PR. Currently you can rebase:

git chechout <branch>
git pull --rebase PowerShell master
git push -f

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Travis Plunk (TravisEz13) · 2018-03-14T01:14:58Z

This PR appears empty. Can it be closed?

Tim Parkinson (timparkinson) · 2018-03-14T17:21:40Z

Closing as well out of date.

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Aug 5, 2017

Ilya (iSazonov) reviewed Aug 5, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) assigned Ilya (iSazonov) Aug 17, 2017

Steve Lee (SteveL-MSFT) added Review - Committee The PR/Issue needs a review from the PowerShell Committee Breaking-Change breaking change that may affect users labels Aug 25, 2017

Steve Lee (SteveL-MSFT) added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Aug 25, 2017

Tim Parkinson (timparkinson) mentioned this pull request Sep 4, 2017

Get-Date -UFormat inconsistencies compared to Linux strftime #4750

Closed

8 of 14 tasks

Tim Parkinson and others added 8 commits September 4, 2017 12:01

Fix Get-Date -UFormat '%V' week number handling.

f13a8b5

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

Revert "Fix Get-Date -UFormat '%V' week number handling."

407eb51

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Fix Get-Date -UFormat '%V' week number handling.

38eb1f7

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

Revert "Fix Get-Date -UFormat '%V' week number handling."

f952830

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Fix Get-Date -UFormat '%V' week number handling.

59af68e

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

Revert "Fix Get-Date -UFormat '%V' week number handling."

7955192

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Fix Get-Date -UFormat '%V' week number handling.

b1e4cf9

Fix PowerShell#4264 makes week number handling compliant with ISO-8601.

Revert "Fix Get-Date -UFormat '%V' week number handling."

a23b81a

This reverts commit a5886cd5e419e07b884815f740e08b3c69513179. Solution was incorrect and needs a lot more work.

Tim Parkinson (timparkinson) force-pushed the master branch from 277c796 to a23b81a Compare September 4, 2017 11:02

Microsoft GitHub User (msftgits) removed the cla-already-signed label Sep 26, 2017

PowerShell Team (PowerShell) deleted a comment from Microsoft Contribution License Agreements (msftclas) Sep 27, 2017

Jason Shirk (lzybkr) removed their request for review October 10, 2017 02:08

Travis Plunk (TravisEz13) changed the title ~~Fix Get-Date -UFormat '%V' week number output~~ Fix Get-Date -UFormat %V week number output Mar 14, 2018

Travis Plunk (TravisEz13) changed the title ~~Fix Get-Date -UFormat %V week number output~~ WIP: Fix Get-Date -UFormat %V week number output Mar 14, 2018

Tim Parkinson (timparkinson) closed this Mar 14, 2018

Ilya (iSazonov) mentioned this pull request Apr 4, 2018

Some fixes in Get-Date -UFormat #6542

Merged

5 of 11 tasks

Conversation

Tim Parkinson (timparkinson) commented Aug 5, 2017 • edited by TravisEz13 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Aug 5, 2017

Choose a reason for hiding this comment

Uh oh!

Tim Parkinson (timparkinson) Aug 7, 2017

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Aug 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) Aug 14, 2017

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Aug 5, 2017

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) commented Aug 18, 2017

Uh oh!

Tim Parkinson (timparkinson) commented Aug 18, 2017

Uh oh!

Ilya (iSazonov) commented Aug 21, 2017

Uh oh!

Tim Parkinson (timparkinson) commented Aug 22, 2017

Uh oh!

Ilya (iSazonov) commented Aug 22, 2017

Uh oh!

Joey Aiello (joeyaiello) commented Aug 25, 2017

Uh oh!

Steve Lee (SteveL-MSFT) commented Aug 25, 2017

Uh oh!

Ilya (iSazonov) commented Sep 4, 2017

Uh oh!

Tim Parkinson (timparkinson) commented Sep 4, 2017

Uh oh!

Ilya (iSazonov) commented Sep 4, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Mar 14, 2018

Uh oh!

Tim Parkinson (timparkinson) commented Mar 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Tim Parkinson (timparkinson) commented Aug 5, 2017 •

edited by TravisEz13

Loading

Ilya (iSazonov) Aug 7, 2017 •

edited

Loading

Ilya (iSazonov) Aug 14, 2017 •

edited

Loading