[RFC] Properly handle keyword + COMMENT keyword

[Nicolas Goaziou <n.goaziou@gmail.com>]

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

Hello,

COMMENT keyword is not always clearly defined in Org. Some parts
consider it is a regular keyword as treat it as such (e.g. `org-todo')
whereas some others see it as an additional keyword (e.g.,
`org-priority').

I think the latter makes more sense, and, as a consequence, Org Syntax
(http://orgmode.org/worg/dev/org-syntax.html) defines it that way. Much
like :ARCHIVE:, COMMENT provides an additional property to the headline,
without limiting it whatsoever.

In other words, the following should be valid:

  * TODO COMMENT Headline

and, according to Org Syntax, so should this:

  * TODO [#A] COMMENT Headline

Though, COMMENT keyword must come after any other keyword and priority,
if any. So the following is /not/ valid:

  * COMMENT TODO Headline

Thus, this patch

 - properly fontifies headlines with both a regular keyword and
   a COMMENT keyword,

 - fixes `org-toggle-comment' and `org-todo' to handle both COMMENT
   keyword and another one

 - adds some consistency to functions implementing their own COMMENT
   matching (e.g., with or without case-sensitivity).


WDYT?

-- 
Nicolas Goaziou

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-COMMENT-keyword-when-stacked-with-a-regular-keyw.patch --]
[-- Type: text/x-diff, Size: 6846 bytes --]

From 0a16a1136dbda9deec40b9a72e7dd0a5d648db71 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <n.goaziou@gmail.com>
Date: Mon, 24 Mar 2014 21:46:00 +0100
Subject: [PATCH] Fix COMMENT keyword when stacked with a regular keyword

* lisp/org.el (org-set-font-lock-defaults): Fix headline fontification
  when keywords are stacked.
(org-toggle-comment): Properly toggle COMMENT keyword when a regular
keyword is already present.
(org-todo, org-agenda-prepare-buffers): Correctly match a commented
heading.
* lisp/org-colview.el (org-columns-capture-view): Correctly match
  a commented heading.

* testing/lisp/test-org.el (test-org/toggle-comment): New test.
---
 lisp/org-colview.el      |  8 +++----
 lisp/org.el              | 40 +++++++++++++++++----------------
 testing/lisp/test-org.el | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 24 deletions(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 07d140f..9ebea98 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -1201,8 +1201,6 @@ containing the title row and all other rows.  Each row is a list
 of fields."
   (save-excursion
     (let* ((title (mapcar 'cadr org-columns-current-fmt-compiled))
-	   (re-comment (format org-heading-keyword-regexp-format
-			       org-comment-string))
 	   (re-archive (concat ".*:" org-archive-tag ":"))
 	   (n (length title)) row tbl)
       (goto-char (point-min))
@@ -1214,9 +1212,9 @@ of fields."
 				 (/ (1+ (length (match-string 1))) 2)
 			       (length (match-string 1)))))
 		     (get-char-property (match-beginning 0) 'org-columns-key))
-	    (when (save-excursion
-		    (goto-char (point-at-bol))
-		    (or (looking-at re-comment)
+	    (when (or (org-in-commented-heading-p t)
+		      (save-excursion
+			(goto-char (point-at-bol))
 			(looking-at re-archive)))
 	      (org-end-of-subtree t)
 	      (throw 'next t))
diff --git a/lisp/org.el b/lisp/org.el
index ef0bc3f..94bda2a 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -6343,9 +6343,11 @@ needs to be inserted at a specific position in the font-lock sequence.")
 	   ;; Code
 	   '(org-activate-code (1 'org-code t))
 	   ;; COMMENT
-	   (list (format org-heading-keyword-regexp-format
-			 (concat "\\(" org-comment-string "\\)"))
-		 '(2 'org-special-keyword t))
+	   (list (format
+		  "^\\*\\(?: +%s\\)?\\(?: +\\[#[A-Z0-9]\\]\\)? +\\(?9:%s\\)\\(?: \\|$\\)"
+		  org-todo-regexp
+		  org-comment-string)
+		 '(9 'org-special-keyword t))
 	   ;; Blocks and meta lines
 	   '(org-fontify-meta-lines-and-blocks))))
     (setq org-font-lock-extra-keywords (delq nil org-font-lock-extra-keywords))
@@ -12176,17 +12178,17 @@ expands them."
   (interactive)
   (save-excursion
     (org-back-to-heading)
-    (let (case-fold-search)
-      (cond
-       ((looking-at (format org-heading-keyword-regexp-format
-			    org-comment-string))
-	(goto-char (match-end 1))
-	(looking-at (concat " +" org-comment-string))
-	(replace-match "" t t)
-	(when (eolp) (insert " ")))
-       ((looking-at org-outline-regexp)
-	(goto-char (match-end 0))
-	(insert org-comment-string " "))))))
+    (looking-at org-complex-heading-regexp)
+    (goto-char (or (match-end 3) (match-end 2) (match-end 1)))
+    (skip-chars-forward " \t")
+    (unless (memq (char-before) '(?\s ?\t)) (insert " "))
+    (if (org-in-commented-heading-p t)
+	(delete-region (point)
+		       (progn (search-forward " " (line-end-position) 'move)
+			      (skip-chars-forward " \t")
+			      (point)))
+      (insert org-comment-string)
+      (unless (eolp) (insert " ")))))
 
 (defvar org-last-todo-state-is-todo nil
   "This is non-nil when the last TODO state change led to a TODO state.
@@ -12301,7 +12303,7 @@ When called through ELisp, arg is also interpreted in the following way:
       (save-excursion
 	(catch 'exit
 	  (org-back-to-heading t)
-	  (when (looking-at (concat "^\\*+ " org-comment-string))
+	  (when (org-in-commented-heading-p t)
 	    (org-toggle-comment)
 	    (setq commentp t))
 	  (if (looking-at org-outline-regexp) (goto-char (1- (match-end 0))))
@@ -18229,11 +18231,11 @@ When a buffer is unmodified, it is just killed.  When modified, it is saved
 		   (if (org-at-heading-p t)
 		       (add-text-properties (point-at-bol) (org-end-of-subtree t) pa))))
 	       (goto-char (point-min))
-	       (setq re (format org-heading-keyword-regexp-format
-				org-comment-string))
+	       (setq re (format "^\\* .*\\<%s\\>" org-comment-string))
 	       (while (re-search-forward re nil t)
-		 (add-text-properties
-		  (match-beginning 0) (org-end-of-subtree t) pc))))
+		 (when (save-match-data (org-in-commented-heading-p t))
+		   (add-text-properties
+		    (match-beginning 0) (org-end-of-subtree t) pc)))))
 	    (goto-char pos)))))
     (setq org-todo-keywords-for-agenda
           (org-uniquify org-todo-keywords-for-agenda))
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 0144841..9350f93 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -27,6 +27,64 @@
 \f
 ;;; Comments
 
+(ert-deftest test-org/toggle-comment ()
+  "Test `org-toggle-comment' specifications."
+  ;; Simple headline.
+  (should
+   (equal "* Test"
+	  (org-test-with-temp-text "* COMMENT Test"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  (should
+   (equal "* COMMENT Test"
+	  (org-test-with-temp-text "* Test"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  ;; Headline with a regular keyword.
+  (should
+   (equal "* TODO Test"
+	  (org-test-with-temp-text "* TODO COMMENT Test"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  (should
+   (equal "* TODO COMMENT Test"
+	  (org-test-with-temp-text "* TODO Test"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  ;; Empty headline.
+  (should
+   (equal "* "
+	  (org-test-with-temp-text "* COMMENT"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  (should
+   (equal "* COMMENT"
+	  (org-test-with-temp-text "* "
+	    (org-toggle-comment)
+	    (buffer-string))))
+  ;; Headline with a single keyword.
+  (should
+   (equal "* TODO "
+	  (org-test-with-temp-text "* TODO COMMENT"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  (should
+   (equal "* TODO COMMENT"
+	  (org-test-with-temp-text "* TODO"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  ;; Headline with a keyword, a priority cookie and contents.
+  (should
+   (equal "* TODO [#A] Headline"
+	  (org-test-with-temp-text "* TODO [#A] COMMENT Headline"
+	    (org-toggle-comment)
+	    (buffer-string))))
+  (should
+   (equal "* TODO [#A] COMMENT Headline"
+	  (org-test-with-temp-text "* TODO [#A] Headline"
+	    (org-toggle-comment)
+	    (buffer-string)))))
+
 (ert-deftest test-org/comment-dwim ()
   "Test `comment-dwim' behaviour in an Org buffer."
   ;; No region selected, no comment on current line and line not
-- 
1.9.1