Re: Parse malformed clocklines (was: Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx)

[Ihor Radchenko <yantar92@posteo.net>]

Morgan Smith <morgan.j.smith@outlook.com> writes:

>> 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.

> -	    (setq year-end (or (nth 5 date) year-start)
> -		  month-end (or (nth 4 date) month-start)
> -		  day-end (or (nth 3 date) day-start)
> -		  hour-end (or (nth 2 date) (car time-range) hour-start)
> -		  minute-end (or (nth 1 date) (cdr time-range) minute-start))))
> +	    (setq year-end (or (nth 5 date) (and time-range year-start))
> +		  month-end (or (nth 4 date) (and time-range month-start))
> +		  day-end (or (nth 3 date) (and time-range day-start))
> +		  hour-end (or (nth 2 date) (car time-range))
> +		  minute-end (or (nth 1 date) (cdr time-range)))))

This is harmless, but also does nothing - if there is no year, month,
and date specified, `date-end' would never match.

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

Clock lines and timestamps are not only used to calculate clock
sums. They can, for example, be exported. Or they can be used manually,
by reading them as text. So, any kind of change in the parser should be
considered extremely carefully.

> 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.

> 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)

This is not acceptable.
org-element is a _parser_. It has nothing to do with interpretation of
timestamps. Moreover, there is no notion of "invalid" syntax in Org mode
- any text is a valid Org syntax, although it may be interpreted as
plain text if it does not fit any existing markup.

I still insist that it is a job of the Elisp code that interprets the
clock lines (the clock sum function) to display warnings and otherwise
analyze the timestamp components. We may also hint potential problems in
org-lint.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>