Skip to content

Update Duration.prototype.round to latest spec #188

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

Closed
sffc opened this issue Jan 28, 2025 · 8 comments · Fixed by #317
Closed

Update Duration.prototype.round to latest spec #188

sffc opened this issue Jan 28, 2025 · 8 comments · Fixed by #317
Assignees
Labels
C-internal Internal library improvements E-help wanted Extra attention is needed
Milestone

Comments

@sffc
Copy link
Contributor

sffc commented Jan 28, 2025

This function appears to be written against an old version of the spec. It is hard to verify correctness and update it to use the newer AOs.

@Magnus-Fjeldstad
Copy link
Contributor

// 27. If plainRelativeTo is not undefined, then
                // a. Let internalDuration be ToInternalDurationRecordWith24HourDays(duration).
                let time_duration = self.time.to_normalized().add_days(self.days().as_())?;
                // b. Let targetTime be AddTime(MidnightTimeRecord(), internalDuration.[[Time]]).
                let target_time = PlainTime::default().add_normalized_time_duration(time_duration);
                // c. Let calendar be plainRelativeTo.[[Calendar]].

                // d. Let dateDuration be ! AdjustDateDurationRecord(internalDuration.[[Date]], targetTime.[[Days]]).
                let date_duration = date_duration.adjust(target_time.0.into(), None, None)?;

Started to update the round_with_provider method, within duration.rs.

@nekevss
Copy link
Member

nekevss commented Mar 1, 2025

@Magnus-Fjeldstad Are you still working on this?

@Magnus-Fjeldstad
Copy link
Contributor

Yes but i cant Get the tozonedatetime to work

@nekevss
Copy link
Member

nekevss commented Mar 2, 2025

Sounds good.ToZonedDateTime would be an engine specific implementation item. round should still take the same arguments that it has.

For reference, the Boa implementation is here if you haven't seen it yet. If you have any questions, feel free to reach out in matrix.

@nekevss nekevss added this to the 0.1 Blocking milestone Mar 3, 2025
@Magnus-Fjeldstad
Copy link
Contributor

Thank you, ive been really busy lately but i will try to get some work done the following days. Hopefully a PR by next week.

@nekevss
Copy link
Member

nekevss commented Mar 6, 2025

Sounds good. Feel free to reach out if you have any questions

@nekevss nekevss added the C-internal Internal library improvements label Mar 16, 2025
@Magnus-Fjeldstad
Copy link
Contributor

Hi this have proven to be to difficult for me, i will join @HenrikTennebekk and work on #142. Sorry for the inconvenience.

@nekevss
Copy link
Member

nekevss commented Mar 18, 2025

Totally fine! It's has a lot of various nuances to it.

@nekevss nekevss added the E-help wanted Extra attention is needed label Apr 8, 2025
@HalidOdat HalidOdat self-assigned this May 25, 2025
nekevss added a commit that referenced this issue May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-internal Internal library improvements E-help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants