nikolassv / bartib

A simple timetracker for the command line. It saves a log of all tracked activities as a plaintext file and allows you to create flexible reports.
GNU General Public License v3.0
636 stars 34 forks source link

Ability to round when reporting #31

Closed berkes closed 7 months ago

berkes commented 8 months ago

Thanks for the great application. I love it's simplicity.

Though, I'd love to make it a tad less simple, by introducing a "round" feature.

I build invoices from my timetracking, and I copy it to other time-trackers when clients need that. Many of those have minimum timespans. For invoicing, I never work less than 15 minutes. One system I'm working in, only works with 10 minutes. And so on.

I'd love to add a flag to both report and list that rounds to the nearest whole N.

e.g. `bartib report --round="10m" which would round to 10 minutes. So, say we have:

    guidelines............ 1h 40m
    meeting ..............    29m
    research project scope 1h 12m

And we run it with --round=10m it would show:

    guidelines............ 1h 40m
    meeting ..............    30m
    research project scope 1h 10m

But when we run it with --round=1h it would show:

    guidelines............ 2h
    meeting ..............    0h
    research project scope 1h

Obviously, time tracked is as granular as it is now. Just the output is rounded.

Would this be a feature you'd accept? If so, I'll make a PR for it, and add some tests for it. In that case: should I be aware of any on-going refactorings that I'd best wait for?

nikolassv commented 8 months ago

That sounds like an interesting feature! I could have used it with my previous employers as well. I would more than happy to merge a PR for it. So, please go ahead. Currently, I don't have planned any refactoring that could conflict with the implementation.

berkes commented 8 months ago

Working on it, but I'm running into issues with the way we now pass referenced Activities around.

My idea is as follows:

Right after reading getter::get_activities() we process them. For now, that processing would only be rounding, with a round_activity(&Activity, round: Duration) -> Activity. round_activity returns a new Activity, but with start and end(optional) rounded to nearest Duration. ¹

Processing Activities this early, has as advantage that any calculation, or filter, that follows, uses the rounded values. This way we avoid e.g. filtering for up to 14:15 to e.g. include a entry with, say, end=14:12, but where the rounded value then would show 14:30. That would be weird, not? Same for calculations, e.g. the totals: they should, IMO, be the sum of the rounded values. Not all values summed and then rounded.

The downside of my approach is that in the current code, all filters, processors, views etc, operate on &'a [&'a Activity], not &'a [Activity]. So the change would be rather big: I'd need to refactor all the references to become values. What do you think of this?

¹ Or better: the issue is that I'm not experienced enough in Rust to find a way to return a referenced Activity that isn't the original Activity

nikolassv commented 8 months ago

Your approach sounds reasonable to me. Incorporating a processing step in the reporting and list commands could also help us with issue #29.

Changing the other functions to work on moved values rather than on borrowed values appears to be a significant change. However, at this moment I don't see what would speak against it.