emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ihor Radchenko
Subject: Re: [PATCH] lisp/org-clock.el (org-clock-sum): Rewrite regex using rx
Date: Sat, 13 Apr 2024 16:48:52 +0000

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

>>> -      (goto-line 2)
>>> +      (insert (org-test-clock-create-clock ". 1:00" ". 2:00")
>>> +              "CLOCK: => 1:00\n")
>>
>> This is not a valid clock format. Matching such lines is a bug.
>> See https://list.orgmode.org/orgmode/87wpkkhafc.fsf@saiph.selenimh/
>
> Let me preface this defense with the fact that I don't like this format
> and I don't think we should support it.  Rewriting `org-clock-sum' would
> be much easier if we drop support for it.  However, I do believe we
> currently support it.
>
> First of all, it currently does work.
>
> Accord to the "Version 4.78" release notes as found on worg, this is
> valid.
>
> ```
>    - You may specify clocking times by hand (i.e. without
>      clocking in and out) using this syntax.
>
>      : CLOCK: => 2:00
>
>      Thanks to Scott Jaderholm for this proposal.
> ```

This is convincing. I did not know that this format is explicitly
mentioned in the news.

Our general rule is that we do not drop existing features in Org mode
except extraordinary circumstances:
https://bzg.fr/en/the-software-maintainers-pledge/
Especially when they are documented.

So, in the message I linked, Nicolas (the major Org mode contributor)
was not right. I hence need to fix the parser and update Org syntax
page. This includes fixing `org-element-clock-line-re' to account for
CLOCK: => 1:00 syntax.

Luckily, it does not look like we are going to break the existing
external exporter packages as long as they are using ox.el API -
`org-export-translate' works just fine with missing timestamps.

> Also last time I went to rewrite `org-clock-sum' you said
> (https://list.orgmode.org/orgmode/87bkg7xbxo.fsf@localhost/):
>
> ```
> Further, you dropped the
>
>            ((match-end 4)
>             ;; A naked time.
>
> branch of the code, which accounts for CLOCK: => HH:MM lines that are not 
> clock elements.
> ```

Yup. Although I did not see Nicolas' message that time. My judgment was
simply based on looking at the code and seeing that CLOCK: => HH:MM
matching was clearly intentional.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]