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

Interval#count incorrectly includes the end-point #1306

Closed
diesieben07 opened this issue Oct 19, 2022 · 0 comments · Fixed by #1308
Closed

Interval#count incorrectly includes the end-point #1306

diesieben07 opened this issue Oct 19, 2022 · 0 comments · Fixed by #1308

Comments

@diesieben07
Copy link
Collaborator

diesieben07 commented Oct 19, 2022

Describe the bug
The Interval documentation states that

An Interval object represents a half-open interval of time

To me this means that the interval includes the starting point, but not the end point. This is confirmed by checking interval.contains(interval.end), which always returns false. However Interval#count also counts the endpoint.

To Reproduce

const {Interval, DateTime} = require('luxon');

const startDate = DateTime.fromISO("2022-10-01T00:00:00.000+02:00");
const interval = Interval.after(startDate, {days: 2});

console.log(interval.contains(interval.end)); // as expected this is false
console.log(interval.count('days')); // this reports 3, but the interval only contains 2 calendar days (1st and 2nd)
console.log(interval.length('hours')); // as expected this reports exactly 48, because the first millisecond of the 3rd is not included

See here: https://runkit.com/embed/xxwq2ofg22di

Actual vs Expected behavior
Interval#count should not count the interval's endpoint.

Desktop (please complete the following information):

  • OS: Linux Mint
  • Browser: Chrome 106, also happens on Node 14
  • Luxon version: 3.0.4
  • Your timezone: Europe/Berlin

Additional context
Looking at the code for count, it unconditionally adds 1 to the result of diffing start and end. This addition must only be done if end is actually different from end.startOf(unit).

diesieben07 added a commit to diesieben07/luxon that referenced this issue Oct 19, 2022
diesieben07 added a commit to diesieben07/luxon that referenced this issue Oct 19, 2022
icambron pushed a commit that referenced this issue Mar 4, 2023
* Fix Interval#count counting the endpoint as part of the interval
Fixes #1306

* Run formatter
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

Successfully merging a pull request may close this issue.

1 participant