1076: fromFormat should prefer the IANA zone if there are multiple zone specifiers

[brendan-gero-humanetix]

This PR is an attempt to address this issue: #1076
I've tried to take the approach of minimising modifications to the public API. I'm not completely confident in some of the changes this has led to, especially in the fromSQL function.

[linux-foundation-easycla]

• ❌ The commit (41b83b6 ,9a2cacfe218e5b4637dd435e056ae545022ff3c5 ,5e735f9a8c9522c4be79b6d52f042202afac171c ,2d36d7d4b54a50ca02341815b1d67e9ad6b87dc7 ,73d303e99583413ac29251a0766b9ff374e813d1) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

[brendan-gero-humanetix]

Could I get some help with the issue with the EasyCLA check?

• My Github profile has my email address
• I've signed the OpenJS Foundation Corporate CLA as my employer's Initial CLA Manager
• My email address matches the domain that I have added under "Approved List Of Contributors From My Organization" in the Corporate CLA Console.

I have just tried clicking the "NOT COVERED" button, and selecting the Corporate CLA option. I saw a message saying that this project does not support the Corporate CLA. Is that just an oversight?

[linux-foundation-easycla]

• ❌ The commit (41b83b6 ,9a2cacfe218e5b4637dd435e056ae545022ff3c5 ,5e735f9a8c9522c4be79b6d52f042202afac171c ,2d36d7d4b54a50ca02341815b1d67e9ad6b87dc7 ,73d303e99583413ac29251a0766b9ff374e813d1) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

[icambron]

I'm just going to accept the CLA signature as-is. I think you've done it correctly; I'm not sure why the automation isn't accepting it. But it's good by me.

The change itself I have some feedback on. So first, a mea culpa: I had misunderstood your question in the issue thread and answered too quickly. I didn't realize you were positioning it to use the numerical offset to interpret and then converting to the IANA zone. My mistake.

I had in mind just changing the precedence to:

  let zone;
  if (!isUndefined(matches.z)) {
    zone = IANAZone.create(matches.z);
  } else if (!isUndefined(matches.Z)) {
    zone = new FixedOffsetZone(matches.Z);
  } else {
    zone = null;
  }

That's it; in other words, I'm suggesting totally ignoring the numerical offset.

Looking at the tests, I think I see why you did the more elaborate change: it allows the parser to resolve ambiguous DST times using the offset. I can see the merits of that.

But the semantics here are too complicated: using setZone: true, we interpret using the offset and convert to the IANA zone. I get why that's a convenient way to implement the ambiguous-interpretation, but that's just too messy, and you can see how awkward it sounds in the docstrings. In particular, the test DateTime.fromFormat() converts offset not belonging to time zone is too strange. That should give 9 for hour, not 6, because it should ignore the erroneous 11. I should have caught that in your first question (again, my mistake).

So here's a proposal to get the best of both worlds:

1. If one is there but not the other (doesn't matter which), then use that for both interpretation and (if setZone is true) conversion
2. If both are there, use IANA as the authority for both interpretation and conversion over the numeric offset
3. If both are there but the time is ambiguous, use the numerical offset to inform which side of the ambiguity is taken

3 sounds hard but really isn't. Here's my version: #1083

[brendan-gero-humanetix]

@icambron Sorry I haven't gotten back to you sooner, I've gotten a bit distracted.

I think your PR does make sense. I'd be tempted to go one step further and say that a timestamp with an offset that is completely different to the IANAZone should actually be invalid. However, I don't think I'll be dealing with timestamps like that in my situation, so I'd personally be happy if your PR was accepted as is.

Should we reject this PR then?