Ihor Radchenko <yantar92@posteo.net> writes:

> Morgan Smith <morgan.j.smith@outlook.com> writes:
>
>>> That's expected.
>>> We have the following _syntax_ description for clock lines:
>>>
>>> https://orgmode.org/worg/org-syntax.html#Clocks...>> clock: INACTIVE-TIMESTAMP-RANGE DURATION
>> ...
>> My specific issue is that the ":*-end" stuff can be set when the
>> "*-start" stuff is not.
>> ...
>> CLOCK: [2023-11-15 Wed 15rr:26]--[2023-11-15 Wed 16:12] =>  0:46
>> calculated time: 58320.0
>> ...
>> What this shows is that an invalid end will cause the entry to be
>> ignored, but an invalid start will not.
>
> Yup. Everything makes sense, but only within syntax spec. Actual code
> that handles such clock lines is another story.
>
> Warning is important because a number of people use clock functionality
> for billing - miscalculating clock sums can literally affect people's
> income :)
>
> I think that the best course of action when a problematic timestamp
> without opening/closing time is encountered is:
>
> 1. Warn user
> 2. Still calculate the duration, assuming 0s in time (simply because
>    previous versions of Org mode did it)
>
> (2) is kind of debatable, but I can imagine some users might make use of
>
> such feature by putting clocks by hand:
> CLOCK: [2023-11-15 Wed]--[2023-11-16 Thu] => 24:00
>
> These users may then simply suppress the warning.

I'm very tempted to just make it hard rule that clocklines need to be
fully specified.  If you take a look at the attached patch, it shows one
way we could do this.

First, I modify the timestamp parser so the ":*-end" variables don't
always inherit the ":*-start" variables.  They can be nil independent of
the start.

Then in the clockline parser I ensure that every single field is set.
If it isn't, then I set the timestamp to nil so it won't be used.

This preserves the flexibility you want in timestamp parsing while also
catching many malformed clocklines.

Let me know what you think of this approach.

Also a couple questions about this approach:

1. Will changing the timestamp parser have far-reaching implications?
Is this something I should avoid?

2. Is there a way to give user errors in the parser code?  I'm using
`org-element--cache-warn' in my patch but I'm not sure that's the right
thing to do.  (if it is the right thing then we need to define it
earlier in the file)