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