Introduce an option to round to neares N minutes or hours.

[berkes]

Please consider my initial attempt at a rounding feature.

Some thoughts:

• I've ignored rounding over days. While e.g. --round 5d might be neat for reports or invoices that work over a week, it opens up a lot of difficulties. Users can still round by --round 120h which achieves something similar but is probably expected to act a bit weird.

• As proposed in #31, I went for an architecture that injects Processors based on the arguments. We now have one processor, so the architecture is probably overkill. But it may be useful. Please let me know if you wish to get that in a separate commit or separate PR even. I've now decided that its easiest to just clump everything in one commit.

• As discussed in #31, because of the processing, we need brand new DateTime objects rather than references to them as they come from the file. After all: we create different times on reading. This makes the PR rather larger than I'm confortable with, as it touches a lot of methods that used to pass references (with lifetimes) around and now pass values and/or copies of them around. I suspect that this impacts performance for everyone even without using the --round feature.

I'm not a very confident Rust developer (yet ;) so feel free to roast my (ab)use, or lack of, lifetimes, references and such.

This fixes #31

[nikolassv]

Thank you, this looks really promising! I believe we can use the concept of processors to solve #29 as well.

As soon as I am able, I will have a closer look at it!

[berkes]

Sorry for the linting issue. Apparently I had a somewhat outdated fmt and/or clippy. I've now fixed that to match what the CI uses.

[nikolassv]

I haven't looked at the implementation details yet, but I assume that the PR may have a conceptional issue since it rounds start and end times instead of durations. This approach could lead to unexpected results.

For instance, let's consider two activities A and B. Activity A started at 9:08 and ended at 9:52, while activity B started at 10:07 and ended at 10:38. Without rounding, activity A has a duration of 44 Minutes and activity B a duration of 31 minutes. However, after rounding to 15 minutes bartib reports a duration of 30 minutes for activity A and 45 minutes for activity B. This is due to the fact that we round the start end end time of A to 9:15 and 9:45 respectively, and those of B to 10:00 and 10:45. As a result, the shorter activity is reported as being longer.

You can use this example file as a reference:

2023-11-17 09:08 - 2023-11-17 09:52 | Project A | A longer activity
2023-11-17 10:07 - 2023-11-17 10:38 | Project B | A shorter activity

Do you believe it would be possible to round for durations instead of start and end times? Perhaps it would be feasible to do the rounding in three steps:

1. Calculate the rounded duration
2. Round the start time
3. Set the end time by adding the rounded duration to the start time

What are your thoughts on this?

[berkes]

Ah, I understand. But that was deliberate. I guess we have found two distinct use-cases for rounding. And I think we need them both, to avoid confusion. And that both need to get a distinctive name.

I understand your use-case.

My case:

• I have external time-reporting software where I must track time by putting start and end time.
  • One uses increments of 10 minutes. So 13:00, 13:10, 13:20 and so forth. I cannot put 13:02.
  • One uses ranges of four hours. So [_] morning (09:00-13:00) [_] afternoon (14:00-18:00)

Without thinking too hard about it (which I should've, apparently ;) I went for "rounding is what I need". Conceptually it's more of a "snap to T" than actual rounding.

I previously used timetrap for my time tracking. And it's rounding uses the method of snapping the start and end as well

But, indeed, I also have the use-case where I simply want to invoice at least 15 minutes; so where the duration of the report and list should become rounded. So I supposedly need both.

I guess the features would need to be:

• Round start and end times (this patch)
• Round totals (your suggestion)

I'll spend some time to implement both. Would that work for you?

And if so, what should we name them, and their commandline arguments? Some ideas I have are --round-start-end, versus --round-duration, but that's probably a bit too technical.

[nikolassv]

Thank you, I hadn't realized that there may be two distinct types of rounding that are relevant. If I view it from this perspective, it seems there's quite a list of requirements that a perfect rounding feature would fulfill:

1. It rounds start and end times
2. It rounds duration
3. It maintains the order of duration such that the longest activity before rounding is the longest after rounding
4. It does not create artificial pauses in a continuous series of activities
5. The sum of duration after rounding does not differ by more than the rounding period from the original sum of duration.

Your way of rounding (snapping start and end times) does indeed fulfill all requirements except the third. The method which I have suggested does fulfill the first three, but can violate the fourth and fifth.

I am not aware of any refined rounding algorithm that would fulfill all five requirements. So, I believe we should proceed with the method that you have implemented. I don't think there is a need for my suggested rounding method.

Would you mind adding examples to the "Command Overview" section in the README for the usage of the rounding feature?

[berkes]

The sum of duration after rounding does not differ by more than the rounding period from the original sum of duration.

I think we might be able to solve this by treating "start-end" rounding separate from rounding of aggregates (Sum being the only one I see now).

So, basically, we'd need to pass the rounded and the non-rounded items along. Probably in the Activity. The obvious (and IMO) ugly pattern would be to introduce original_start/end attrs. I think, however, that a derived_from: Box<Activity> attribute inside the Activity may work better. Not sure how the borrow and lifetime checker would like this though. In any case, some (smart) pointer to the activity as it was logged. We might need separate Structs, like ProcessedActivity or RawActivity, one that then has the pointer to the other. Again: IDK the details wrt Rust, yet.

Then, on the moment of reporting, the reporter, can decide which one to use for its reporting. It gets both. It can decide "I need to use the processed version for this" or "For the totals, I need the unprocessed version".

I might be overthinking all this though. I'm trying to generalize it, but cannot think of a use-case beyond rounding yet. The "split across days" version, probably doesn't need such pointers to the original, AFAICS.

Would you mind adding examples to the "Command Overview" section in the README for the usage of the rounding feature?

Certainly!

Then, WRT testing, I noticed there wasn't a very obvious place for an integration test. I'd love to add some integration tests that tick all the boxes you placed above. Any thoughts on this?

And lastly: I've a rather big backlog of work these days, and don't expect that to slow down. I'm spending some free hours on this, but please don't expect this within days. Sorry for that.

[nikolassv]

Adding some derivedFrom property to the Activity struct and passing it around could be a solution. However, it seems as if it would add too much complexity for such a limited use case. I believe rounding the start and end times, like you implemented in your PR, is the right thing to do at the moment. If the need for any further refinement arises in the future, it can be added then.

If you could simply add the feature to the README, I'm going to merge the PR.

[berkes]

I've added it to the README and made clear, in both the --help and the README that it's rounding the start and end time.

[nikolassv]

Thank you very much for your work and this PR! This is a cool feature and I am happy to merge it.