Browse Source

Throw an error when trying to set invalid properties

* lisp/org.el (org--valid-property-p): New function.
(org-entry-put):
(org-set-property): Use new function.

Suggested-by: Julien Cubizolles <j.cubizolles@free.fr>
<http://permalink.gmane.org/gmane.emacs.orgmode/104044>
Nicolas Goaziou 9 years ago
parent
commit
22cf1bf7ed
2 changed files with 37 additions and 13 deletions
  1. 30 13
      lisp/org.el
  2. 7 0
      testing/lisp/test-org.el

+ 30 - 13
lisp/org.el

@@ -15559,6 +15559,12 @@ but in some other way.")
   "Some properties that are used by Org mode for various purposes.
   "Some properties that are used by Org mode for various purposes.
 Being in this list makes sure that they are offered for completion.")
 Being in this list makes sure that they are offered for completion.")
 
 
+(defun org--valid-property-p (property)
+  "Non nil when string PROPERTY is a valid property name."
+  (not
+   (or (equal property "")
+       (org-string-match-p "\\s-" property))))
+
 (defun org--update-property-plist (key val props)
 (defun org--update-property-plist (key val props)
   "Associate KEY to VAL in alist PROPS.
   "Associate KEY to VAL in alist PROPS.
 Modifications are made by side-effect.  Return new alist."
 Modifications are made by side-effect.  Return new alist."
@@ -16079,8 +16085,9 @@ and the new value.")
 (defun org-entry-put (pom property value)
 (defun org-entry-put (pom property value)
   "Set PROPERTY to VALUE for entry at point-or-marker POM.
   "Set PROPERTY to VALUE for entry at point-or-marker POM.
 
 
-If the value is nil, it is converted to the empty string.  If
+If the value is nil, it is converted to the empty string.  If it
-it is not a string, an error is raised.
+is not a string, an error is raised.  Also raise an error on
+invalid property names.
 
 
 PROPERTY can be any regular property (see
 PROPERTY can be any regular property (see
 `org-special-properties').  It can also be \"TODO\",
 `org-special-properties').  It can also be \"TODO\",
@@ -16090,7 +16097,9 @@ For the last two properties, VALUE may have any of the special
 values \"earlier\" and \"later\".  The function then increases or
 values \"earlier\" and \"later\".  The function then increases or
 decreases scheduled or deadline date by one day."
 decreases scheduled or deadline date by one day."
   (cond ((null value) (setq value ""))
   (cond ((null value) (setq value ""))
-	((not (stringp value)) (error "Properties values should be strings")))
+	((not (stringp value)) (error "Properties values should be strings"))
+	((not (org--valid-property-p property))
+	 (user-error "Invalid property name: \"%s\"" property)))
   (org-with-point-at pom
   (org-with-point-at pom
     (if (or (not (featurep 'org-inlinetask)) (org-inlinetask-in-task-p))
     (if (or (not (featurep 'org-inlinetask)) (org-inlinetask-in-task-p))
 	(org-back-to-heading t)
 	(org-back-to-heading t)
@@ -16373,21 +16382,29 @@ When use-default, don't even ask, just use the last
 
 
 (defun org-set-property (property value)
 (defun org-set-property (property value)
   "In the current entry, set PROPERTY to VALUE.
   "In the current entry, set PROPERTY to VALUE.
+
 When called interactively, this will prompt for a property name, offering
 When called interactively, this will prompt for a property name, offering
 completion on existing and default properties.  And then it will prompt
 completion on existing and default properties.  And then it will prompt
 for a value, offering completion either on allowed values (via an inherited
 for a value, offering completion either on allowed values (via an inherited
 xxx_ALL property) or on existing values in other instances of this property
 xxx_ALL property) or on existing values in other instances of this property
-in the current file."
+in the current file.
+
+Throw an error when trying to set a property with an invalid name."
   (interactive (list nil nil))
   (interactive (list nil nil))
-  (let* ((property (or property (org-read-property-name)))
+  (let ((property (or property (org-read-property-name))))
-	 (value (or value (org-read-property-value property)))
+    ;; `org-entry-put' also makes the following check, but this one
-	 (fn (cdr (assoc property org-properties-postprocess-alist))))
+    ;; avoids polluting `org-last-set-property' and
-    (setq org-last-set-property property)
+    ;; `org-last-set-property-value' needlessly.
-    (setq org-last-set-property-value (concat property ": " value))
+    (unless (org--valid-property-p)
-    ;; Possibly postprocess the inserted value:
+      (user-error "Invalid property name: \"%s\"" property))
-    (when fn (setq value (funcall fn value)))
+    (let ((value (or value (org-read-property-value property)))
-    (unless (equal (org-entry-get nil property) value)
+	  (fn (cdr (assoc-string property org-properties-postprocess-alist t))))
-      (org-entry-put nil property value))))
+      (setq org-last-set-property property)
+      (setq org-last-set-property-value (concat property ": " value))
+      ;; Possibly postprocess the inserted value:
+      (when fn (setq value (funcall fn value)))
+      (unless (equal (org-entry-get nil property) value)
+	(org-entry-put nil property value)))))
 
 
 (defun org-find-property (property &optional value)
 (defun org-find-property (property &optional value)
   "Find first entry in buffer that sets PROPERTY.
   "Find first entry in buffer that sets PROPERTY.

+ 7 - 0
testing/lisp/test-org.el

@@ -3587,6 +3587,13 @@ Paragraph<point>"
   (should-error
   (should-error
    (org-test-with-temp-text "* H\n:PROPERTIES:\n:test: 1\n:END:"
    (org-test-with-temp-text "* H\n:PROPERTIES:\n:test: 1\n:END:"
      (org-entry-put 1 "test" 2)))
      (org-entry-put 1 "test" 2)))
+  ;; Error when property name is invalid.
+  (should-error
+   (org-test-with-temp-text "* H\n:PROPERTIES:\n:test: 1\n:END:"
+     (org-entry-put 1 "no space" "value")))
+  (should-error
+   (org-test-with-temp-text "* H\n:PROPERTIES:\n:test: 1\n:END:"
+     (org-entry-put 1 "" "value")))
   ;; Set "TODO" property.
   ;; Set "TODO" property.
   (should
   (should
    (string-match (regexp-quote " TODO H")
    (string-match (regexp-quote " TODO H")