Skip to content
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.

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

Offset redesign #11

Closed
lifthrasiir opened this issue Nov 30, 2014 · 0 comments
Closed

Offset redesign #11

lifthrasiir opened this issue Nov 30, 2014 · 0 comments
Assignees

Comments

@lifthrasiir
Copy link
Contributor

The current Offset is problematic because we conflate the offset state and offset conversion. Consequently we cannot convert to Local (!). This is because Local is stored alongside the NaiveDateTime and should contain a cached FixedOffset (otherwise every method would slow down).

We cannot simply strip the offset state down, or replace it with the mandatory FixedOffset. Okay, no-overhead DateTime<UTC> might be an illusion, but some Offset implementation would want something other than FixedOffset: Tzfile (#10), for example, would want to store the offset to the most recent transition instead.

Associated types do not help either. For example struct DateTime<Off: Offset> { dt: NaiveDateTime, st: Off::State, off: Off } wouldn't work (duh). It seems that we may have to accept that we should use different types for conversion (.with_offset(...)) and state (DateTime<...>).

@lifthrasiir lifthrasiir self-assigned this Nov 30, 2014
lifthrasiir added a commit that referenced this issue Jan 12, 2015
- We have splitted `Offset` into `Offset` and `OffsetState` (name
  changes in consideration). The former is used to construct and convert
  local or UTC date, and the latter is used to store the UTC offset
  inside constructed values. Some offsets are their own states as well.

- This uses lots of associated types which implementation is still in
  flux. Currently it crashes with debuginfo enabled. We've temporarily
  disabled debuginfo from `Cargo.toml`.

- This technically allows a conversion to the local time, but not yet
  tested.
lifthrasiir added a commit that referenced this issue Feb 4, 2015
Basically, this should close #11 when officially released.

- Formatting syntax is now refactored out of the rendering logic.
  The main syntax is available in the `format::strftime` module,
  which also serves as a documentation for the syntax.

- A parser (modelled after `strptime(3)`) has been implemented.
  See the individual commits for the detailed implementation.

- There are two ways to get a timezone-aware value from a string:
  `Offset` or `DateTime<FixedOffset>`. The former should be used
  when the offset is known in advance (e.g. assume the local date)
  while the latter should be used when the offset is unknown.
  Naive types have a simple `from_str` method.

- There are some known problems with the parser (even after
  tons of tests), which will be sorted out in 0.2. Known issues:

  - This does not exactly handle RFC 2822 and RFC 3339, which
    subtly differs from the current implementation in
    case-sensitivity, whitespace handling and legacy syntax.
    I'd like to integrate #24 for this cause.

  - Time zone names are not recognized at all. There is even
    no means to get a name itself, not sure about the resolution.

  - `Parsed` does *not* constrain `year` to be negative,
    so manually prepared `Parsed` may give a negative year.
    But the current verification pass may break such cases.

  - I absolutely don't know about the parser's performance!

- `AUTHORS.txt` has been added, for what it's worth.
lifthrasiir added a commit that referenced this issue Feb 18, 2015
- We have splitted `Offset` into `Offset` and `OffsetState` (name
  changes in consideration). The former is used to construct and convert
  local or UTC date, and the latter is used to store the UTC offset
  inside constructed values. Some offsets are their own states as well.

- This uses lots of associated types which implementation is still in
  flux. Currently it crashes with debuginfo enabled. We've temporarily
  disabled debuginfo from `Cargo.toml`.

- This technically allows a conversion to the local time, but not yet
  tested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant