Hello, Samuel Loury <konubi...@gmail.com> writes:
> I would like to submit a tiny patch to make use of `org-clock-string' > instead of the hard coded value CLOCK: whenever possible. Thank you. Some comments follow. > From 8eedb019d277f7f1e8baa6641244ddf7e298d397 Mon Sep 17 00:00:00 2001 > From: Konubinix <konubi...@gmail.com> > Date: Tue, 26 Aug 2014 09:11:23 +0200 > Subject: [PATCH] Make use of the constant `org-clock-string' whenever possible > > Instead of the hardcoded value "CLOCK:". > > * lisp/org-clock.el (org-find-open-clocks) > * lisp/org.el (org-clone-subtree-with-time-shift) > * lisp/org.el (org-insert-property-drawer) > * lisp/org.el (org-at-clock-log-p) The commit message looks strange. What about [PATCH] Use `org-clock-string' whenever possible * lisp/org-clock.el (org-find-open-clocks): * lisp/org.el (org-clone-subtree-with-time-shift, org-insert-property-drawer, org-at-clock-log-p): Use `org-clock-string' whenever possible instead of hardcoded "CLOCK". > TINYCHANGE Since you signed FSF papers, you don't need to add "TINYCHANGE" anymore. > - (while (re-search-forward "CLOCK: \\(\\[.*?\\]\\)$" nil t) > + (while (re-search-forward (concat org-clock-string " \\(\\[.*?\\]\\)$") > nil t) You are building a new string before each search, which is sub-optimal. (let ((re (concat org-clock-string " \\(\\[.*?\\]\\)$"))) (while (re-search-forward re nil t) ...)) > - (while (re-search-forward "^[ \t]*CLOCK:.*$" nil t) > + (while (re-search-forward > + (format "^[ \t]*%s.*$" org-clock-string) nil t) Ditto. > - (while (looking-at "^[ \t]*\\(:CLOCK:\\|:LOGBOOK:\\|CLOCK:\\|:END:\\)") > - (if (member (match-string 1) '("CLOCK:" ":END:")) > + (while (looking-at > + (format > + "^[ \t]*\\(:CLOCK:\\|:LOGBOOK:\\|%s\\|:END:\\)" > + org-clock-string)) Ditto. > + (if (member (match-string 1) > + (list org-clock-string ":END:")) > - (looking-at "^[ \t]*CLOCK:"))) You are building a new list each time. > + (looking-at > + (concat "^[ \t]*" org-clock-string)))) See above. Anyway this would benefit from a rewrite using "org-element.el" (*Hint*) but that's a much bigger task. Regards, -- Nicolas Goaziou