slaesvuo/hledger
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix:timeclock: process in parse order; fully support overlapping sessions [#2417]

Browse Source 
We no longer attempt to process timeclock entries in time order - that
was a wrong requirement, probably given by me, that can't work.  Now
we just process them in parse order. This plus a little tweaking of
error checking fixes several ordering bugs with overlapping sessions
and also allows same-named overlapping sessions.

More cleanup will follow. More testing might show that --old-timeclock
is no longer needed.
This commit is contained in:
Simon Michael 2025-09-01 08:11:47 +01:00
parent 7ac0fa1aaa
commit 9849f196cb
4 changed files with 72 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
hledger-lib/Hledger/Data/Timeclock.hs
View File

@ -79,8 +79,8 @@ timeclockToTransactionsOld :: LocalTime -> [TimeclockEntry] -> [Transaction]
timeclockToTransactionsOld _ [] = [] timeclockToTransactionsOld _ [] = []
timeclockToTransactionsOld now [i] timeclockToTransactionsOld now [i]
| tlcode i /= In = errorExpectedCodeButGot In i | tlcode i /= In = errorExpectedCodeButGot In i
| odate > idate = entryFromTimeclockInOut i o' : timeclockToTransactionsOld now [i',o] | odate > idate = entryFromTimeclockInOut True i o' : timeclockToTransactionsOld now [i',o]
| otherwise = [entryFromTimeclockInOut i o] | otherwise = [entryFromTimeclockInOut True i o]
where where
o = TimeclockEntry (tlsourcepos i) Out end "" "" "" [] o = TimeclockEntry (tlsourcepos i) Out end "" "" "" []
end = if itime > now then itime else now end = if itime > now then itime else now
@ -91,8 +91,8 @@ timeclockToTransactionsOld now [i]
timeclockToTransactionsOld now (i:o:rest) timeclockToTransactionsOld now (i:o:rest)
| tlcode i /= In = errorExpectedCodeButGot In i | tlcode i /= In = errorExpectedCodeButGot In i
| tlcode o /= Out = errorExpectedCodeButGot Out o | tlcode o /= Out = errorExpectedCodeButGot Out o
| odate > idate = entryFromTimeclockInOut i o' : timeclockToTransactionsOld now (i':o:rest) | odate > idate = entryFromTimeclockInOut True i o' : timeclockToTransactionsOld now (i':o:rest)
| otherwise = entryFromTimeclockInOut i o : timeclockToTransactionsOld now rest | otherwise = entryFromTimeclockInOut True i o : timeclockToTransactionsOld now rest
where where
(itime,otime) = (tldatetime i,tldatetime o) (itime,otime) = (tldatetime i,tldatetime o)
(idate,odate) = (localDay itime,localDay otime) (idate,odate) = (localDay itime,localDay otime)
@ -105,16 +105,18 @@ timeclockToTransactionsOld now (i:o:rest)
-- It allows concurrent clocked-in sessions (though not with the same account name), -- It allows concurrent clocked-in sessions (though not with the same account name),
-- and clock-in/clock-out entries in any order. -- and clock-in/clock-out entries in any order.
-- --
-- Entries are processed in time order, then (for entries with the same time) in parse order. -- Entries are processed in parse order.
-- When there is no clockout, one is added with the provided current time.
-- Sessions crossing midnight are split into days to give accurate per-day totals. -- Sessions crossing midnight are split into days to give accurate per-day totals.
-- At the end, any sessions with no clockout get an implicit clockout with the provided "now" time.
-- If any entries cannot be paired as expected, an error is raised. -- If any entries cannot be paired as expected, an error is raised.
-- --
timeclockToTransactions :: LocalTime -> [TimeclockEntry] -> [Transaction] timeclockToTransactions :: LocalTime -> [TimeclockEntry] -> [Transaction]
timeclockToTransactions now entries = transactions timeclockToTransactions now entries0 = transactions
where where
sessions = dbg6 "sessions" $ pairClockEntries (sortTimeClockEntries entries) [] [] -- don't sort by time, it messes things up; just reverse to get the parsed order
transactionsFromSession s = entryFromTimeclockInOut (in' s) (out s) entries = dbg7 "timeclock entries" $ reverse entries0
sessions = dbg6 "sessions" $ pairClockEntries entries [] []
transactionsFromSession s = entryFromTimeclockInOut False (in' s) (out s)
-- If any "in" sessions are in the future, then set their out time to the initial time -- If any "in" sessions are in the future, then set their out time to the initial time
outtime te = max now (tldatetime te) outtime te = max now (tldatetime te)
createout te = TimeclockEntry (tlsourcepos te) Out (outtime te) (tlaccount te) "" "" [] createout te = TimeclockEntry (tlsourcepos te) Out (outtime te) (tlaccount te) "" "" []
@ -122,11 +124,6 @@ timeclockToTransactions now entries = transactions
stillopen = dbg6 "stillopen" $ pairClockEntries ((active sessions) <> outs) [] [] stillopen = dbg6 "stillopen" $ pairClockEntries ((active sessions) <> outs) [] []
transactions = map transactionsFromSession $ sortBy (\s1 s2 -> compare (in' s1) (in' s2)) (completed sessions ++ completed stillopen) transactions = map transactionsFromSession $ sortBy (\s1 s2 -> compare (in' s1) (in' s2)) (completed sessions ++ completed stillopen)
-- | Sort timeclock entries first by date and time (with time zone ignored as usual), then by file position.
-- Ie, sort by time, but preserve the parse order of entries with the same time.
sortTimeClockEntries :: [TimeclockEntry] -> [TimeclockEntry]
sortTimeClockEntries = sortBy (\e1 e2 -> compare (tldatetime e1, tlsourcepos e1) (tldatetime e2, tlsourcepos e2))
-- | Assuming that entries have been sorted, we go through each time log entry. -- | Assuming that entries have been sorted, we go through each time log entry.
-- We collect all of the "i" in the list "actives," and each time we encounter -- We collect all of the "i" in the list "actives," and each time we encounter
-- an "o," we look for the corresponding "i" in actives. -- an "o," we look for the corresponding "i" in actives.
@ -161,16 +158,6 @@ timeclockToTransactions now entries = transactions
<> [ "Overlapping sessions with the same account name are not supported." ] <> [ "Overlapping sessions with the same account name are not supported." ]
-- XXX better to show full session(s) -- XXX better to show full session(s)
-- <> map T.show (filter ((`elem` activesinthisacct).in') sessions) -- <> map T.show (filter ((`elem` activesinthisacct).in') sessions)
where
makeTimeClockErrorExcerpt :: TimeclockEntry -> T.Text -> T.Text
makeTimeClockErrorExcerpt e@TimeclockEntry{tlsourcepos=pos} msg = T.unlines [
T.pack (sourcePosPretty pos) <> ":"
,l <> " | " <> T.show e
-- ,T.replicate (T.length l) " " <> " |" -- <> T.replicate c " " <> "^")
] <> msg
where
l = T.show $ unPos $ sourceLine $ tlsourcepos e
-- c = unPos $ sourceColumn $ tlsourcepos e
-- | Find the relevant clockin in the actives list that should be paired with this clockout. -- | Find the relevant clockin in the actives list that should be paired with this clockout.
-- If there is a session that has the same account name, then use that. -- If there is a session that has the same account name, then use that.
@ -208,12 +195,22 @@ errorExpectedCodeButGot expected actual = error' $ printf
l = show $ unPos $ sourceLine $ tlsourcepos actual l = show $ unPos $ sourceLine $ tlsourcepos actual
c = unPos $ sourceColumn $ tlsourcepos actual c = unPos $ sourceColumn $ tlsourcepos actual
makeTimeClockErrorExcerpt :: TimeclockEntry -> T.Text -> T.Text
makeTimeClockErrorExcerpt e@TimeclockEntry{tlsourcepos=pos} msg = T.unlines [
T.pack (sourcePosPretty pos) <> ":"
,l <> " | " <> T.show e
-- ,T.replicate (T.length l) " " <> " |" -- <> T.replicate c " " <> "^")
] <> msg
where
l = T.show $ unPos $ sourceLine $ tlsourcepos e
-- c = unPos $ sourceColumn $ tlsourcepos e
-- | Convert a timeclock clockin and clockout entry to an equivalent journal -- | Convert a timeclock clockin and clockout entry to an equivalent journal
-- transaction, representing the time expenditure. Note this entry is not balanced, -- transaction, representing the time expenditure. Note this entry is not balanced,
-- since we omit the \"assets:time\" transaction for simpler output. -- since we omit the \"assets:time\" transaction for simpler output.
entryFromTimeclockInOut :: TimeclockEntry -> TimeclockEntry -> Transaction entryFromTimeclockInOut :: Bool -> TimeclockEntry -> TimeclockEntry -> Transaction
entryFromTimeclockInOut i o entryFromTimeclockInOut requiretimeordered i o
| otime >= itime = t | not requiretimeordered || otime >= itime = t
| otherwise = | otherwise =
-- Clockout time earlier than clockin is an error. -- Clockout time earlier than clockin is an error.
-- (Clockin earlier than preceding clockin/clockout is allowed.) -- (Clockin earlier than preceding clockin/clockout is allowed.)
@ -260,8 +257,13 @@ entryFromTimeclockInOut i o
-- since otherwise it will often have large recurring decimal parts which (since 1.21) -- since otherwise it will often have large recurring decimal parts which (since 1.21)
-- print would display all 255 digits of. timeclock amounts have one second resolution, -- print would display all 255 digits of. timeclock amounts have one second resolution,
-- so two decimal places is precise enough (#1527). -- so two decimal places is precise enough (#1527).
amt = mixedAmount $ setAmountInternalPrecision 2 $ hrs hours amt = case mixedAmount $ setAmountInternalPrecision 2 $ hrs hours of
ps = [posting{paccount=acctname, pamount=amt, ptype=VirtualPosting, ptransaction=Just t}] a | not $ a < 0 -> a
_ -> error' $ printf
"%s%s:\nThis clockout is earlier than the clockin."
(makeTimeClockErrorExcerpt i "")
(makeTimeClockErrorExcerpt o "")
ps = [posting{paccount=acctname, pamount=amt, ptype=VirtualPosting, ptransaction=Just t}]
-- tests -- tests
@ -292,13 +294,13 @@ tests_Timeclock = testGroup "Timeclock" [
step "use the clockin time for auto-clockout if it's in the future" step "use the clockin time for auto-clockout if it's in the future"
txndescs [clockin future "" "" "" []] @?= [printf "%s-%s" futurestr futurestr] txndescs [clockin future "" "" "" []] @?= [printf "%s-%s" futurestr futurestr]
step "multiple open sessions" step "multiple open sessions"
txndescs txndescs (reverse [
[ clockin (mktime today "00:00:00") "a" "" "" [], clockin (mktime today "00:00:00") "a" "" "" [],
clockin (mktime today "01:00:00") "b" "" "" [], clockin (mktime today "01:00:00") "b" "" "" [],
clockin (mktime today "02:00:00") "c" "" "" [], clockin (mktime today "02:00:00") "c" "" "" [],
clockout (mktime today "03:00:00") "b" "" "" [], clockout (mktime today "03:00:00") "b" "" "" [],
clockout (mktime today "04:00:00") "a" "" "" [], clockout (mktime today "04:00:00") "a" "" "" [],
clockout (mktime today "05:00:00") "c" "" "" [] clockout (mktime today "05:00:00") "c" "" "" []
] ])
@?= ["00:00-04:00", "01:00-03:00", "02:00-05:00"] @?= ["00:00-04:00", "01:00-03:00", "02:00-05:00"]
] ]

14
hledger/hledger.m4.md
View File

@ -4720,9 +4720,9 @@ of [timeclock.el](http://www.emacswiki.org/emacs/TimeClock).
As with [Ledger](http://ledger-cli.org/3.0/doc/ledger3.html#Time-Keeping), As with [Ledger](http://ledger-cli.org/3.0/doc/ledger3.html#Time-Keeping),
hledger's timeclock format is a subset/variant of timeclock.el's. hledger's timeclock format is a subset/variant of timeclock.el's.
Note, hledger's timeclock format was made more robust in hledger 1.43 and 1.50. hledger's timeclock format was updated in hledger 1.43 and 1.50.
If your old time logs are rejected, you should adapt them to modern hledger; If your old time logs are rejected, you should adapt them to modern hledger;
but for now you can also restore the pre-1.43 behaviour with the `--old-timeclock` flag. for now, you can restore the pre-1.43 behaviour with the `--old-timeclock` flag.
Here the timeclock format in hledger 1.50+: Here the timeclock format in hledger 1.50+:
@ -4768,8 +4768,11 @@ $ hledger -f a.timeclock print
``` ```
Clock-ins and clock-outs are matched by their account/session name. Clock-ins and clock-outs are matched by their account/session name.
If a clock-outs does not specify a name, the most recent unclosed clock-in is closed. If a clock-out does not specify a name, the most recent unclosed clock-in is closed.
Also, sessions spanning more than one day are automatically split at day boundaries. You can have multiple sessions active simultaneously.
Entries are processed in the order they are parsed.
Sessions spanning more than one day are automatically split at day boundaries.
Eg, the following time log: Eg, the following time log:
```timeclock ```timeclock
@ -4804,9 +4807,6 @@ $ hledger -f t.timeclock print
``` ```
Note, you can have overlapping sessions (multiple sessions open simultaneously), but they must have different account names.
Overlapping sessions with the same account name are currently not supported currently.
Here is a Here is a
[sample.timeclock](https://raw.github.com/simonmichael/hledger/master/examples/sample.timeclock) to [sample.timeclock](https://raw.github.com/simonmichael/hledger/master/examples/sample.timeclock) to
download and some queries to try: download and some queries to try:

7
hledger/test/errors/tcclockouttime.test
View File

@ -1,8 +1,3 @@
$$$ hledger check -f tcclockouttime.timeclock $$$ hledger check -f tcclockouttime.timeclock
>>>2 /Error: .*tcclockouttime.timeclock:5:1: >>>2 /This clockout is earlier than the clockin./
5 \| o 2022-01-01 00:00:00
\| \^
Could not find previous clockin to match this clockout.
/
>>>= 1 >>>= 1

29
hledger/test/timeclock.test
View File

@ -192,7 +192,7 @@ $ hledger -f timeclock:- print
>= >=
# ** 14. Sessions with the same account name may not overlap (#2417). # ** 14. Sessions can overlap, even with the same account name (#2417).
< <
i 2024-01-01 13:00:00 a i 2024-01-01 13:00:00 a
o 2024-01-01 15:00:00 o 2024-01-01 15:00:00
@ -200,10 +200,31 @@ i 2024-01-01 14:00:00 a
o 2024-01-01 15:00:00 o 2024-01-01 15:00:00
$ hledger -f timeclock:- print $ hledger -f timeclock:- print
>2 /Overlapping sessions with the same account name are not supported./ 2024-01-01 * 13:00-15:00
>=!0 (a) 2.00h
# ** 15. --old-timeclock also affects included files. 2024-01-01 * 14:00-15:00
(a) 1.00h
>=
# ** 15. These staggered overlapping sessions are read correctly.
<
i 2000-01-01 09:00:00 a
o 2000-01-01 12:00:00
i 2000-01-01 10:00:00 b
o 2000-01-01 13:00:00
$ hledger -f timeclock:- print
2000-01-01 * 09:00-12:00
(a) 3.00h
2000-01-01 * 10:00-13:00
(b) 3.00h
>=
# ** 16. --old-timeclock also affects included files.
< <
include needs-old.timeclock include needs-old.timeclock