Allow removing the zeroes of durations

[JKamue]

Changes

With this merge request the internal removeZeroes function of a Duration will also be accessible publically.

Why?

If you end up having zero values within your duration they are almost impossible to "destruct" with the current state of the Api.
At the moment the only Option is to use rescale(). But this is not compatible / wont help with some of the other use cases offered by the Api.

One concrete example of that is the following:

1. I want to parse a duration and output it in a way that only displays year, month and days if they are not zero
2. At the same time I want to aggregate years and months (I dont want 24 months for example)

Try#1 - shiftTo()

Intuitively one would use shiftTo() like this:

// Expectation: {"days": 8, "years": 2}
Duration.fromObject({ months: 24, days: 8 }).shiftTo("years", "months", "days").toObject();
// Reality: {"days": 8, "months": 0, "years": 2}

When using something like toHuman() this will produce an output like 2 years, 0 months, 8 days which I dont want to show to the user. This solution breaks rule#1

Try#2 - rescale()

rescale() is the only function currently capable of removing zero values.

// Expectation: {"days": 8, "years": 2}
Duration.fromObject({ months: 24, days: 8 }).rescale().toObject();
// Reality: {"days": 1, "weeks": 1, "years": 2}

This breaks rule#1 and displays units other than year, month and days .

Try#3 - Best of both worlds?

Duration.fromObject({ months: 24, days: 8 }).rescale().shiftTo("years", "months", "days").toObject();
// {"days": 1, "months": 0, "years": 2}
// shiftTo will always create years and months, even if you only input { days: 8 }

Duration.fromObject({ months: 24, days: 8 }).shiftTo("years", "months", "days").rescale().toObject();
// {"days": 1, "weeks": 1, "years": 2}

Either combination does not work.

Solution with this MR

Duration.fromObject({ months: 24, days: 8 }).shiftTo("years", "months", "days").removeZeroes().toObject()
// {"days": 8, "years": 2}

Adding removeZeroes() at the end will get rid of the leftover 0 months created by shiftTo().
In a real world example using toHuman() would produce a proper output like 2 years, 8 days.

Alternatives

As shown there is no alternative to really get rid of something like 0 months or years if you want to shift to years or months.
There are two possible alternatives but the offer less flexbility to the user:

1. Add the option to exclude 0 to toHuman()
2. Add the option to exclude 0 to shiftTo()

But overall I think the ability to remove zeroes whenever wanted warrents the addition of this function.

I am open to feedback, let me know what you think!
I had some troubles generating the docs, so I am happy if you can help me out there.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: JKamue / name: Jannes Kaspar-Müller (1d50002, c894bd1, ba6a301)

[diesieben07]

Thank you for your contribution and the thorough analysis!
I agree with your assessment of a dedicated method allowing greater user flexibility and I think that should be kept. However I think also that the use-case you have described is so common for toHuman that it warrants adding an option there. I would suggest showZeros which defaults to true. What do you think?

[JKamue]

Thanks for your kind reply!
I agree that most likely removeZeroes().toHuman() would be a common thing to do so the addition as a parameter makes sense and setting showZeroes to true by default will not create any breaking api changes.

I will implement this tomorrow and update the pull request.

Is there other things you think the PR might lack? As mentioned previously I was unable to auto generate the documentation.
Does the code and test coverage fit your expectations?

[JKamue]

@diesieben07 I amended better tests for removeZeroes() to the first commit 1d50002 and I added the option to hide zeroes with showZeroes: false when using toHuman() in the second commit ba6a301.

Let me know what you think!

[diesieben07]

I am very very sorry for the delayed review.

Thank you for the changes, the showZeros option looks fine except for the default handling that I noted.

[JKamue]

No worries and thanks for the feedback.
I implemented all the requested changes and added some tests👍