[PATCH] Make use of the constant `org-clock-string' whenever possible

[PATCH] Make use of the constant `org-clock-string' whenever possible

[Samuel Loury]

[-- Attachment #1.1: Type: text/plain, Size: 418 bytes --]

Hi,

I would like to submit a tiny patch to make use of `org-clock-string'
instead of the hard coded value CLOCK: whenever possible.

I also found out that :CLOCK: was hard coded in
`org-insert-property-drawer' (git sha
67ae102b4be87976240555d1c0d80ee55906f53c, file lisp/org.el:15897)
instead of using `org-clock-drawer-start-re' but I did not have time to
fix this one.

Please find the patch attached to this mail.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Make-use-of-the-constant-org-clock-string-whenever-p.patch --]
[-- Type: text/x-diff, Size: 2748 bytes --]

From 8eedb019d277f7f1e8baa6641244ddf7e298d397 Mon Sep 17 00:00:00 2001
From: Konubinix <konubinix@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)

TINYCHANGE
---
 lisp/org-clock.el |  2 +-
 lisp/org.el       | 14 ++++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 179d395..14b613e 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -799,7 +799,7 @@ If CLOCK-SOUND is non-nil, it overrides `org-clock-sound'."
     (with-current-buffer buf
       (save-excursion
 	(goto-char (point-min))
-	(while (re-search-forward "CLOCK: \\(\\[.*?\\]\\)$" nil t)
+	(while (re-search-forward (concat org-clock-string " \\(\\[.*?\\]\\)$") nil t)
 	  (push (cons (copy-marker (match-end 1) t)
 		      (org-time-string-to-time (match-string 1))) clocks))))
     clocks))
diff --git a/lisp/org.el b/lisp/org.el
index 750b9d1..4559280 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -8698,7 +8698,8 @@ and still retain the repeater to cover future instances of the task."
 			    (org-entry-delete nil "ID")
 			  (org-id-get-create t)))
 	    (unless (= n 0)
-	      (while (re-search-forward "^[ \t]*CLOCK:.*$" nil t)
+	      (while (re-search-forward
+		      (format "^[ \t]*%s.*$" org-clock-string) nil t)
 		(kill-whole-line))
 	      (goto-char (point-min))
 	      (while (re-search-forward drawer-re nil t)
@@ -15891,8 +15892,12 @@ formats in the current buffer."
     (setq hiddenp (outline-invisible-p))
     (end-of-line 1)
     (and (equal (char-after) ?\n) (forward-char 1))
-    (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))
+      (if (member (match-string 1)
+		  (list org-clock-string ":END:"))
 	  ;; just skip this line
 	  (beginning-of-line 2)
 	;; Drawer start, find the end
@@ -17589,7 +17594,8 @@ With prefix ARG, change that many days."
   "Is the cursor on the clock log line?"
   (save-excursion
     (move-beginning-of-line 1)
-    (looking-at "^[ \t]*CLOCK:")))
+    (looking-at
+     (concat "^[ \t]*" org-clock-string))))
 
 (defvar org-clock-history)                     ; defined in org-clock.el
 (defvar org-clock-adjust-closest nil)          ; defined in org-clock.el
-- 
2.1.0.rc1


[-- Attachment #1.3: Type: text/plain, Size: 10 bytes --]

--
Samuel

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Nicolas Goaziou]

Hello,

Samuel Loury <konubinix@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 <konubinix@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

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Samuel Loury]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Hi,

Thank you for the comments, I will try to improve the commit as soon as
possible.

About org-element.el, I would be happy to try. I guess the best
documentation is the file itself (it appears to be quite well
commented). Is it a good way to start?

-- 
Konubinix
GPG Key    : 7439106A
Fingerprint: 5993 BE7A DA65 E2D9 06CE  5C36 75D2 3CED 7439 106A

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Nicolas Goaziou]

Hello,

Samuel Loury <konubinix@gmail.com> writes:

> About org-element.el, I would be happy to try. I guess the best
> documentation is the file itself (it appears to be quite well
> commented). Is it a good way to start?

There are also bits of documentation here and there, on the ML, in the
"ox.el" reference... I'm in the process of writing a small document
collecting these bits.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Marcin Borkowski]

Dnia 2014-08-27, o godz. 09:34:16
Nicolas Goaziou <mail@nicolasgoaziou.fr> napisał(a):

> There are also bits of documentation here and there, on the ML, in the
> "ox.el" reference... I'm in the process of writing a small document
> collecting these bits.

Wow, that would be cool!  I tried to look into the docstrings in
org-element.el, but found them a bit intimidating.  Especially that I
know very little about Org's internal data structures.

> Regards,

Best,

-- 
Marcin Borkowski
http://octd.wmi.amu.edu.pl/en/Marcin_Borkowski
Adam Mickiewicz University

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Nicolas Goaziou]

Hello,

Marcin Borkowski <mbork@wmi.amu.edu.pl> writes:

> Dnia 2014-08-27, o godz. 09:34:16
> Nicolas Goaziou <mail@nicolasgoaziou.fr> napisał(a):
>
>> There are also bits of documentation here and there, on the ML, in the
>> "ox.el" reference... I'm in the process of writing a small document
>> collecting these bits.
>
> Wow, that would be cool!  I tried to look into the docstrings in
> org-element.el, but found them a bit intimidating.  Especially that I
> know very little about Org's internal data structures.

Done at

  http://orgmode.org/worg/dev/org-element-api.html


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Marcin Borkowski]

Dnia 2014-08-31, o godz. 21:56:19
Nicolas Goaziou <mail@nicolasgoaziou.fr> napisał(a):

> >> There are also bits of documentation here and there, on the ML, in
> >> the "ox.el" reference... I'm in the process of writing a small
> >> document collecting these bits.
> >
> > Wow, that would be cool!  I tried to look into the docstrings in
> > org-element.el, but found them a bit intimidating.  Especially that
> > I know very little about Org's internal data structures.
> 
> Done at
> 
>   http://orgmode.org/worg/dev/org-element-api.html

Great!  Sent to my Kindle.

> Regards,

Best,

-- 
Marcin Borkowski
http://octd.wmi.amu.edu.pl/en/Marcin_Borkowski
Adam Mickiewicz University

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Samuel Loury]

[-- Attachment #1.1: Type: text/plain, Size: 395 bytes --]

Hi,

Finally, I don't have time to dig into org-element.el for the time
being.

Nonetheless, please find attached a revised version of the patch with
your comments taken into account.

In order to avoid creating the regexp each time `org-at-clock-log-p' is
called, I added the constant `org-clock-line-re', following the same
naming convention as `org-planning-or-clock-line-re' defined above.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-PATCH-Use-org-clock-string-whenever-possible.patch --]
[-- Type: text/x-diff, Size: 3918 bytes --]

From d0247756c9a9c8a753dfba6c2de0c4f1d9ed4fc5 Mon Sep 17 00:00:00 2001
From: Konubinix <konubinix@gmail.com>
Date: Tue, 26 Aug 2014 09:11:23 +0200
Subject: [PATCH] [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".
 Add the `org-clock-line-re' regexp matching a line with clock info.
---
 lisp/org-clock.el |  3 ++-
 lisp/org.el       | 21 +++++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 179d395..2ffcbfa 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -795,11 +795,12 @@ If CLOCK-SOUND is non-nil, it overrides `org-clock-sound'."
   "Search through the given file and find all open clocks."
   (let ((buf (or (get-file-buffer file)
 		 (find-file-noselect file)))
+	(org-clock-re (concat org-clock-string " \\(\\[.*?\\]\\)$"))
 	clocks)
     (with-current-buffer buf
       (save-excursion
 	(goto-char (point-min))
-	(while (re-search-forward "CLOCK: \\(\\[.*?\\]\\)$" nil t)
+	(while (re-search-forward org-clock-re nil t)
 	  (push (cons (copy-marker (match-end 1) t)
 		      (org-time-string-to-time (match-string 1))) clocks))))
     clocks))
diff --git a/lisp/org.el b/lisp/org.el
index 750b9d1..2dd3e80 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -396,6 +396,10 @@ a timestamp with \\[org-schedule].")
   "Matches a line with planning or clock info.
 Matched keyword is in group 1.")
 
+(defconst org-clock-line-re
+  (concat "^[ \t]*" org-clock-string)
+  "Matches a line with clock info.")
+
 ;;;; Drawer
 
 (defconst org-drawer-regexp "^[ \t]*:\\(\\(?:\\w\\|[-_]\\)+\\):[ \t]*$"
@@ -8659,6 +8663,7 @@ and still retain the repeater to cover future instances of the task."
 	       ""))) ;; No time shift
 	(n-no-remove -1)
 	(drawer-re org-drawer-regexp)
+	(org-clock-re (format "^[ \t]*%s.*$" org-clock-string))
 	beg end template task idprop
 	shift-n shift-what doshift nmin nmax)
     (if (not (and (integerp n) (> n 0)))
@@ -8698,7 +8703,8 @@ and still retain the repeater to cover future instances of the task."
 			    (org-entry-delete nil "ID")
 			  (org-id-get-create t)))
 	    (unless (= n 0)
-	      (while (re-search-forward "^[ \t]*CLOCK:.*$" nil t)
+	      (while (re-search-forward
+		      org-clock-re nil t)
 		(kill-whole-line))
 	      (goto-char (point-min))
 	      (while (re-search-forward drawer-re nil t)
@@ -15883,6 +15889,10 @@ formats in the current buffer."
 		  0))
 	(beg (point))
 	(re (concat "^[ \t]*" org-keyword-time-regexp))
+	(org-clock-re (format
+	     "^[ \t]*\\(:CLOCK:\\|:LOGBOOK:\\|%s\\|:END:\\)"
+	     org-clock-string))
+	(org-skip-line-list (list org-clock-string ":END:"))
 	end hiddenp)
     (outline-next-heading)
     (setq end (point))
@@ -15891,8 +15901,10 @@ formats in the current buffer."
     (setq hiddenp (outline-invisible-p))
     (end-of-line 1)
     (and (equal (char-after) ?\n) (forward-char 1))
-    (while (looking-at "^[ \t]*\\(:CLOCK:\\|:LOGBOOK:\\|CLOCK:\\|:END:\\)")
-      (if (member (match-string 1) '("CLOCK:" ":END:"))
+    (while (looking-at
+	    org-clock-re)
+      (if (member (match-string 1)
+		  org-skip-line-list)
 	  ;; just skip this line
 	  (beginning-of-line 2)
 	;; Drawer start, find the end
@@ -17589,7 +17601,8 @@ With prefix ARG, change that many days."
   "Is the cursor on the clock log line?"
   (save-excursion
     (move-beginning-of-line 1)
-    (looking-at "^[ \t]*CLOCK:")))
+    (looking-at
+     org-clock-line-re)))
 
 (defvar org-clock-history)                     ; defined in org-clock.el
 (defvar org-clock-adjust-closest nil)          ; defined in org-clock.el
-- 
2.1.0.rc1


[-- Attachment #1.3: Type: text/plain, Size: 104 bytes --]

-- 
Konubinix
GPG Key    : 7439106A
Fingerprint: 5993 BE7A DA65 E2D9 06CE  5C36 75D2 3CED 7439 106A

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH] Make use of the constant `org-clock-string' whenever possible

[Nicolas Goaziou]

Samuel Loury <konubinix@gmail.com> writes:

> Nonetheless, please find attached a revised version of the patch with
> your comments taken into account.

Applied (with some spurious newlines removed). Thank you.


Regards,

-- 
Nicolas Goaziou