icambron / twix.js

:hourglass::left_right_arrow: A date range plugin for moment.js
https://isaaccambron.com/twix.js/
MIT License
379 stars 54 forks source link

Fixed `split()` ending prematurely due to reversed sort order. #95

Closed wbercx closed 7 years ago

wbercx commented 7 years ago

This pull request fixes an issue we were having when splitting a Twix range, where one of the split points equals the end of the original range. In the diagram below, our original range is: From: 16-11-2016 16:00 to 17-11-2016 0:00 on Split points: 16-11-2016 18:00 and 17-11-2016 0:00.

                 18:00                     0:00
                   |                        |
    +---------------------------------------+
    |              |                        |
    +---------------------------------------+
  16:00                                    0:00

twix.coffee:170 performs the following: times = (mom for mom in times when mom.isValid() && mom >= start).sort()

A simplified version of the result:

[
    [0] => '17-11-2016 00:00',
    [1] => '16-11-2016 16:00'
]

Splitting then starts, starting with the first entry and stops because this split point equals our original range's end time. It never gets around to splitting at 16:00.

I have added a compare function to .sort(). This ensures the split points are sorted in ascending order, and split does not end prematurely any longer.

Added a test case as well!

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 97.933% when pulling eaaffd3beeda2c95f6b663b9ccf557fd82a6a439 on wbercx:master into 4df624fe8ae8adda21abdc159954f1fecfac2f85 on icambron:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 97.933% when pulling fe0f7ea3c32044417e2332b5ffd424f60115f036 on wbercx:master into 4df624fe8ae8adda21abdc159954f1fecfac2f85 on icambron:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 97.933% when pulling 3ad162f7b694d2b346dbe2d2d40d07bc305a78c7 on wbercx:master into 4df624fe8ae8adda21abdc159954f1fecfac2f85 on icambron:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 97.933% when pulling 3ad162f7b694d2b346dbe2d2d40d07bc305a78c7 on wbercx:master into 4df624fe8ae8adda21abdc159954f1fecfac2f85 on icambron:master.

icambron commented 7 years ago

Looks good. Thanks for the contribution!