emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [RFC] Rewrite `org-entry-properties' using parser
@ 2014-07-31 23:21 Thorsten Jolitz
  2014-08-01 11:44 ` Bastien
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Thorsten Jolitz @ 2014-07-31 23:21 UTC (permalink / raw)
  To: emacs-orgmode


Hi List,

here is my first take of rewriting `org-entry-properties'.

The existing function predates the new parser and some Org variables,
and thus does the parsing and the property classification itself. The
new version leaves parsing to the parser and property classification
(mostly) to existing Org variables, resulting in much simpler code. 

OTOH, the new version offers more fine-grained control over property
selection. I was a bit unhappy when the use of property-drawers as
simple key-val databases and meta-data stores for USERS was kind of
deprecated with the introduction of the new parser (in favor of usage by
the SYSTEM). This improved with introduction of the 'prop:t' export
option, and the new version of `org-entry-properties' should be powerful
and convenient enough to separate user and system data stored in the
same property-drawers.

Implementation goals were:

 1. (almost) full backward-compability. The parser upcases user
   properties, thus case-sensitivity is lost after parsing and old
   applications that rely on the difference between "foo", "Foo" and
   "FOO" as property keys will break

 2. allow retrieving all property-classes defined in Org-mode separately

 3. allow filtering out parser-specific properties

 4. allow retrieving all "non-org" properties (user and application defined)

 5. allow retrieving properties by regexp-matching, e.g. props prefixed
   with "foo_" by application 'foo'. 

I did not bother to prepare a patch yet since this should be reviewed
and tested:

 - Are some options useless?

 - Do the return values of the options make sense?

 - Is property-classification consistent (e.g. "TODO" in
   `org-special-properties', but :todo-keyword and :todo-type in the
   parse-tree)?

 - I actually reimplemented the docstring of the old function instead of
   the rather complicated code - did I get the semantics right?

 - are there bugs?

 - etc ...

Here is an Org file with the new version of `org-entry-properties',
helper functions and some 20 ERT-tests. Please have a look.


* org-entry-properties
** new function (org.el)

#+begin_src emacs-lisp
(defun org-entry-properties (&optional pom which specific)
  "Get all properties of the entry at point-or-marker POM.

This includes the TODO keyword, the tags, time strings for
deadline, scheduled, and clocking, and any additional properties
defined in the entry.  

The return value is an alist, except if WHICH has value `parser',
then a plist filtered for the properties set by
`org-element-parse-headline' is returned. Keys may occur multiple
times if the property key was used several times.  POM may also
be nil, in which case the current entry is used.

WHICH can have several meaningful values:

 - nil or `all' :: get all regular (non parser) properties

 - `special' :: get properties that are member of
   `org-special-properties'

 - `standard' :: get properties of that subclass

 - `parser' :: get properties set by parser (as plist)

 - `custom' :: get properties that are member of
   `org-custom-properties'

 - `default' :: get properties that are member of
   `org-default-properties'

 - `document' :: get properties that are member of
   `org-element-document-properties'

 - `file' :: get properties that are member of
   `org-file-properties'

 - `global' :: get properties that are member of
   `org-global-properties'

 - `global-fixed' :: get properties that are member of
   `org-global-properties-fixed'

 - `non-org' :: get properties that are not member of any of the
   preceeding classes (except `all')

 - any string :: get only exactly this property

 - form :: get properties string-matched by (rx-to-string form),
           with FORM being a regular expression in sexp form

SPECIFIC can be a string, symbol or keyword, all types will be
converted to an upcased string. It is the specific property we
are interested in. This argument only exists for historical
reasons and backward portability, since giving a string value to
WHICH has the same effect as giving a value to SPECIFIC. However,
if SPECIFIC is non-nil, it takes precedence over WHICH."
  (setq which (or which 'all))
  (org-with-wide-buffer
   (org-with-point-at pom
     (when (and (derived-mode-p 'org-mode)
		(ignore-errors (org-back-to-heading t)))
       (let ((elem (org-element-at-point)))
	 (when (eq (car elem) 'headline)
	   (let* ((specific-prop
		   (cond
		    ((or (org-string-nw-p specific)
			 (org-string-nw-p which))
		     (upcase
		      (or (org-string-nw-p specific)
			  (org-string-nw-p which))))
		    ((keywordp specific)
		     (car (org-split-string
			   (format "%s" specific) ":")))
		    ((and (not (booleanp specific))
			  (symbolp specific))
		     (upcase (format "%s" specific)))
		    (t nil)))
		  (props-plist (cadr elem))
		  (props-alist-strg-keys
		   (org-plist-to-alist props-plist nil t))
		  (parser-keywords
		   (list :raw-value :title :alt-title :begin :end
			 :pre-blank :post-blank :contents-begin
			 :contents-end :level :priority :tags
			 :todo-keyword :todo-type :scheduled
			 :deadline :closed :archivedp :commentedp
			 :footnote-section-p))
		  (parser-keywords-but-special-props
		   (list :raw-value :title :alt-title :begin :end
			 :pre-blank :post-blank :contents-begin
			 :contents-end :level :archivedp
			 :commentedp :footnote-section-p))
		  ;; FIXME necessary?
		  ;; for backward compability only
		  (excluded
		   '("TODO" "TAGS" "ALLTAGS" "PRIORITY" "BLOCKED"))
		  sym)
	     (if specific-prop
		 (assoc specific-prop props-alist-strg-keys)
	       (let ((all-but-parser
		      (progn
			(setplist 'sym props-plist)
			(mapc (lambda (--kw)
				(remprop 'sym --kw))
			      parser-keywords-but-special-props)
			(org-plist-to-alist
			 (symbol-plist 'sym) nil t)))
		     (downcased-special-props
		      (mapcar 'downcase org-special-properties)))
		 (case which
		   ('all all-but-parser)
		   ('non-org (remove-if
			      (lambda (--item)
				(member
				 (car --item)
				 (append
				  org-special-properties
				  org-default-properties
				  org-custom-properties
				  org-element-document-properties
				  org-global-properties
				  org-global-properties-fixed
				  org-file-properties)))
			      all-but-parser))
		   ;; FIXME necessary?
		   ;; for backward compability only
		   ('standard (remove-if
			       (lambda (--item)
				 (member (car --item) excluded))
				 all-but-parser))
		   ;; return plist
		   ('parser (setplist 'sym props-plist)
			    (mapc (lambda (--kw)
				    (remprop 'sym --kw))
				  (set-difference
				   (org-plist-keys props-plist t)
				   parser-keywords))
			    (symbol-plist 'sym))
		   ('special (remove-if-not
			      (lambda (--item)
				(member
				 (car --item)
				 downcased-special-props))
			      props-alist-strg-keys))
		   ('default (remove-if-not
			      (lambda (--item)
				(member (car --item)
					org-default-properties))
			      props-alist-strg-keys))
		   ('custom (remove-if-not
			     (lambda (--item)
			       (member (car --item)
				       org-custom-properties))
			     props-alist-strg-keys))
		   ('document (remove-if-not
			       (lambda (--item)
				 (member
				  (car --item)
				  org-element-document-properties))
			       props-alist-strg-keys))
		   ('global (remove-if-not
			     (lambda (--item)
			       (member (car --item)
				       org-global-properties))
			     props-alist-strg-keys))
		   ('global-fixed (remove-if-not
				   (lambda (--item)
				     (assoc
				      (car --item)
				      org-global-properties-fixed))
				   props-alist-strg-keys))
		   ('file (remove-if-not
			   (lambda (--item)
			     (member (car --item)
				     org-file-properties))
			   props-alist-strg-keys))
		   (t (when (consp which)
			(ignore-errors
			  (let ((rgxp (rx-to-string which)))
			    (remove-if-not
			     (lambda (--item)
			       (string-match rgxp (car --item)))
			     all-but-parser)))))))))))))))
#+end_src


** helper function (org-macs.el)

#+begin_src emacs-lisp
;; copied from kv.el
(defun org-plist-to-alist (plist &optional keys-are-keywords keys-to-string)
  "Convert PLIST to an alist.
The keys are expected to be :prefixed and the colons are removed
unless KEYS-ARE-KEYWORDS is `t'.  The keys in the resulting alist
are symbols unless KEYS-TO-STRING is non-nil."
  (when plist
    (loop for (key value . rest) on plist by 'cddr
	  collect
	  (cons (cond
		 (keys-are-keywords key)
		 (keys-to-string
		  (format "%s" (org-keyword-to-symbol key)))
		 (t (org-keyword-to-symbol key)))
		value))))

;; copied from kv.el
(defun org-alist-keys (alist)
  "Get just the keys from the alist."
  (mapcar (lambda (pair) (car pair)) alist))

;; copied from kv.el
(defun org-alist-values (alist)
  "Get just the values from the alist."
  (mapcar (lambda (pair) (cdr pair)) alist))

(defun org-plist-keys  (plist &optional as-keywords-p)
  "Get just the keys from the plist.
The keys are expected to be :prefixed and the colons are removed
unless AS-KEYWORDS-P is non-nil."
  (org-alist-keys
   (org-plist-to-alist plist as-keywords-p)))

(defun org-plist-values (plist)
  "Get just the values from the plist."
  (org-alist-values
   (org-plist-to-alist plist)))

;; copied from kv.el
(defun org-keyword-to-symbol (keyword)
  "A keyword is a symbol leading with a :.
Converting to a symbol means dropping the :."
  (if (keywordp keyword)
      (intern (substring (symbol-name keyword) 1))
    keyword))
#+end_src


** tests (test-org.el)


#+begin_src emacs-lisp
;;; Properties

(defconst test-org/org-entry-properties-temp-text
"* DONE [#A] headline <2014-07-31 Do> :tag:
  DEADLINE: <2014-08-01 Fr 08:00>
  - State \"DONE\"       from \"WAITING\"    [2014-07-31 Do 22:45]
  - State \"WAITING\"    from \"TODO\"       [2014-07-31 Do 14:46] \\
    testing
  :PROPERTIES:
  :CATEGORY: mycat
  :VISIBILITY_ALL: folded children all
  :foo-key1: val1
  :foo-key2: val2
  :ID:       3996b55d-d678-43a4-af1f-48ed22b5f414
  :CUSTOM_ID: abc123
  :bar:      loo
  :END:
  [2014-07-31 Do 14:45]
"
 "Headline used to test `org-entry-properties'.")

(ert-deftest test-org/org-entry-properties-1 ()
  "Test of `org-entry-properties' specifications."
  (should
  (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("CATEGORY" . "mycat") ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("CUSTOM_ID" . "abc123") ("BAR" . "loo"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil nil nil)))))


(ert-deftest test-org/org-entry-properties-2 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("CATEGORY" . "mycat") ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("CUSTOM_ID" . "abc123") ("BAR" . "loo"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil nil t)))))


(ert-deftest test-org/org-entry-properties-3 ()
  "Test 3 of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil nil 'foo)))))


(ert-deftest test-org/org-entry-properties-4 ()
  "Test 4 of `org-entry-properties' specifications."
  (should
   (equal '("BAR" . "loo")
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil nil "bar")))))

(ert-deftest test-org/org-entry-properties-5 ()
  "Test 5 of `org-entry-properties' specifications."
  (should
   (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("CATEGORY" . "mycat") ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("CUSTOM_ID" . "abc123") ("BAR" . "loo"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil nil '(loo))))))
  
(ert-deftest test-org/org-entry-properties-6 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil t)))))

(ert-deftest test-org/org-entry-properties-7 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'foo)))))

(ert-deftest test-org/org-entry-properties-8 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '("BAR" . "loo")
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil "bar")))))

(ert-deftest test-org/org-entry-properties-9 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("CATEGORY" . "mycat") ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("CUSTOM_ID" . "abc123") ("BAR" . "loo"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'all)))))

(ert-deftest test-org/org-entry-properties-10 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("BAR" . "loo"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'non-org)))))

(ert-deftest test-org/org-entry-properties-11 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("CATEGORY" . "mycat") ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("CUSTOM_ID" . "abc123") ("BAR" . "loo"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'standard)))))

(ert-deftest test-org/org-entry-properties-12 ()
  "Test of `org-entry-properties' specifications."
  (should (equal '(:raw-value "headline <2014-07-31 Do>" :begin 1 :end 448 :pre-blank 0 :contents-begin 44 :contents-end 448 :level 1 :priority 65 :tags ("tag") :todo-keyword "DONE" :todo-type done :deadline (timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)))
		 (org-test-with-temp-text
		     test-org/org-entry-properties-temp-text
		   (org-entry-properties nil 'parser)))))

(ert-deftest test-org/org-entry-properties-13 ()
  "Test of `org-entry-properties' specifications."
  (should (equal '(("priority" . 65) ("tags" "tag") ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)))
		 (org-test-with-temp-text
		     test-org/org-entry-properties-temp-text
		   (org-entry-properties nil 'special)))))

(ert-deftest test-org/org-entry-properties-14 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("CATEGORY" . "mycat") ("CUSTOM_ID" . "abc123"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'default)))))

(ert-deftest test-org/org-entry-properties-15 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'custom)))))

(ert-deftest test-org/org-entry-properties-16 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'document)))))


(ert-deftest test-org/org-entry-properties-17 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'global)))))

(ert-deftest test-org/org-entry-properties-18 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("VISIBILITY_ALL" . "folded children all"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'global-fixed)))))


(ert-deftest test-org/org-entry-properties-19 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil 'file)))))


(ert-deftest test-org/org-entry-properties-20 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal nil
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil '(loo))))))

(ert-deftest test-org/org-entry-properties-21 ()
  "Test of `org-entry-properties' specifications."
  (should
   (equal '(("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2"))
	  (org-test-with-temp-text
	      test-org/org-entry-properties-temp-text
	    (org-entry-properties nil '(and "foo-"))))))

#+end_src


-- 
cheers,
Thorsten

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-07-31 23:21 [RFC] Rewrite `org-entry-properties' using parser Thorsten Jolitz
@ 2014-08-01 11:44 ` Bastien
  2014-08-01 12:44   ` Thorsten Jolitz
  2014-08-01 13:28 ` Nicolas Goaziou
  2014-08-01 18:41 ` Erik Hetzner
  2 siblings, 1 reply; 13+ messages in thread
From: Bastien @ 2014-08-01 11:44 UTC (permalink / raw)
  To: Thorsten Jolitz; +Cc: emacs-orgmode

Hi Thorsten,

Thorsten Jolitz <tjolitz@gmail.com> writes:

> here is my first take of rewriting `org-entry-properties'.

I didn't take the time to closely look at the change, but thanks
for working on this.

My main concern with anything that touches `org-entry-properties'
is whether agenda generation is affected in terms of performances.
Any change here needs to check this very carefully, because we
cannot affort to exchange speed against clarity (sadly enough.)

If you can explore this, that'd be great.

-- 
 Bastien

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-01 11:44 ` Bastien
@ 2014-08-01 12:44   ` Thorsten Jolitz
  0 siblings, 0 replies; 13+ messages in thread
From: Thorsten Jolitz @ 2014-08-01 12:44 UTC (permalink / raw)
  To: emacs-orgmode

Bastien <bzg@gnu.org> writes:

Hi Bastien,

> Thorsten Jolitz <tjolitz@gmail.com> writes:
>
>> here is my first take of rewriting `org-entry-properties'.
>
> I didn't take the time to closely look at the change, but thanks
> for working on this.
>
> My main concern with anything that touches `org-entry-properties'
> is whether agenda generation is affected in terms of performances.
> Any change here needs to check this very carefully, because we
> cannot affort to exchange speed against clarity (sadly enough.)

I would guess it all depends on the speed of `org-element-at-point'
then. I can test this with very big org files later, but first I would
it to converge a bit towards a possibly acceptable version. 

E.g. when looking at the ERT tests it seems ugly and not helpfull that
values of properties like 'deadline' are returned in parse-tree format,
they should be interpreted first (but thats another function-call
affecting speed?).

Furthermore, to achieve real claritiy, the classification of properties
in Org-mode should be checked again for completeness and
consistency. E.g. the 'ID' property set by 'org-id' does not appear in
any 'org--properties' variable and thus would falsely be treated as
user/application property. Properties in 'org-special-properties' are
upcase and don't always match the names of the (downcase) properties set
by the parser. There is no variable for the class of properties that
really only serve the parser/export framework (:beg, :end, :content-beg:
etc.).


> If you can explore this, that'd be great.

As a little test I switched Agenda from day to year mode with
(uncompiled) old and new version. I checked with 

#+begin_src emacs-lisp
(symbol-file 'org-entry-properties)
#+end_src

that uncompiled versions are used, and with 

,----
| C-h f org-entry-properties
`----

which version is loaded. The results are *suspiciously* similar, but the
checks indicated the new version was loaded during second test.

* test-call (in agenda day-view)

#+begin_src emacs-lisp
(benchmark-run nil (org-agenda-change-time-span 'year))
#+end_src

** old (uncompiled)

(54.418716989 94 12.718539017000012)

** new (uncompiled)

(53.862527422 82 12.037524198)

-- 
cheers,
Thorsten

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-07-31 23:21 [RFC] Rewrite `org-entry-properties' using parser Thorsten Jolitz
  2014-08-01 11:44 ` Bastien
@ 2014-08-01 13:28 ` Nicolas Goaziou
  2014-08-05 12:52   ` Thorsten Jolitz
  2014-08-01 18:41 ` Erik Hetzner
  2 siblings, 1 reply; 13+ messages in thread
From: Nicolas Goaziou @ 2014-08-01 13:28 UTC (permalink / raw)
  To: Thorsten Jolitz; +Cc: emacs-orgmode

Hello,

Thorsten Jolitz <tjolitz@gmail.com> writes:

> here is my first take of rewriting `org-entry-properties'.

Interesting. A first round of comments follows.

> Implementation goals were:
>
>  1. (almost) full backward-compability. The parser upcases user
>    properties, thus case-sensitivity is lost after parsing and old
>    applications that rely on the difference between "foo", "Foo" and
>    "FOO" as property keys will break

Case sensitivity never was a feature for properties.

>  - Are some options useless?

Yes, see below.

>  - Is property-classification consistent (e.g. "TODO" in
>    `org-special-properties', but :todo-keyword and :todo-type in the
>    parse-tree)?

"TODO" and :todo-keyword properties are the same.

>  - I actually reimplemented the docstring of the old function instead of
>    the rather complicated code - did I get the semantics right?
>
>  - are there bugs?
>
>  - etc ...
>
> Here is an Org file with the new version of `org-entry-properties',
> helper functions and some 20 ERT-tests. Please have a look.
>
>
> * org-entry-properties
> ** new function (org.el)
>
> #+begin_src emacs-lisp
> (defun org-entry-properties (&optional pom which specific)
>   "Get all properties of the entry at point-or-marker POM.
>
> This includes the TODO keyword, the tags, time strings for
> deadline, scheduled, and clocking, and any additional properties
> defined in the entry.
>
> The return value is an alist, except if WHICH has value `parser',
> then a plist filtered for the properties set by
> `org-element-parse-headline' is returned.

Properties returned by `org-element-at-point' are not meant to be
accessed through `org-entry-properties'. You can remove this option.

>  - nil or `all' :: get all regular (non parser) properties
>
>  - `special' :: get properties that are member of
>    `org-special-properties'
>
>  - `standard' :: get properties of that subclass
>
>  - `parser' :: get properties set by parser (as plist)

See above.

>  - `custom' :: get properties that are member of
>    `org-custom-properties'
>
>  - `default' :: get properties that are member of
>    `org-default-properties'

`org-default-properties' is a (broken) hard-coded list of properties,
only useful for completion. Since this is arbitrary, there's no reason
to check this.

>  - `document' :: get properties that are member of
>    `org-element-document-properties'

The name is misleading, "document properties" are keywords (e.g.
"#+TITLE:"). You can omit this.

>  - `file' :: get properties that are member of
>    `org-file-properties'
>
>  - `global' :: get properties that are member of
>    `org-global-properties'
>
>  - `global-fixed' :: get properties that are member of
>    `org-global-properties-fixed'

The only purpose of `org-global-properties-fixed' is to have a basis for
`org-global-properties'. You can ignore the former.

>  - `non-org' :: get properties that are not member of any of the
>    preceeding classes (except `all')
>
>  - any string :: get only exactly this property
>
>  - form :: get properties string-matched by (rx-to-string form),
>            with FORM being a regular expression in sexp form

Why do you impose `rx-to-string' on the user? Just ask for a regexp. If
one wants exactly a property, he will use `regexp-quote'.

> SPECIFIC can be a string, symbol or keyword, all types will be

keyword type is not needed if you don't access to parser properties.

> converted to an upcased string. It is the specific property we
> are interested in. This argument only exists for historical
> reasons and backward portability, since giving a string value to
> WHICH has the same effect as giving a value to SPECIFIC. However,
> if SPECIFIC is non-nil, it takes precedence over WHICH."
>   (setq which (or which 'all))
>   (org-with-wide-buffer
>    (org-with-point-at pom

`org-with-point-at' already call `org-with-wide-buffer', so you can
remove the latter.

>      (when (and (derived-mode-p 'org-mode)
> 		(ignore-errors (org-back-to-heading t)))
>        (let ((elem (org-element-at-point)))
> 	 (when (eq (car elem) 'headline)

Use `org-element-type', not `car'. Also, you need to include
`inlinetask'

  (when (memq (org-element-type elem) '(headline inlinetask))
    ...)

> 	   (let* ((specific-prop
> 		   (cond
> 		    ((or (org-string-nw-p specific)
> 			 (org-string-nw-p which))
> 		     (upcase
> 		      (or (org-string-nw-p specific)
> 			  (org-string-nw-p which))))
> 		    ((keywordp specific)
> 		     (car (org-split-string
> 			   (format "%s" specific) ":")))

You can ignore `keywordp' part.

> 		    ((and (not (booleanp specific))
> 			  (symbolp specific))
> 		     (upcase (format "%s" specific)))
> 		    (t nil)))

This begs for refactoring. I think

  (specific-prop
   (let ((prop (or specific which)))
     (and prop (upcase (if (stringp prop) prop (symbol-name prop))))))

is sufficient.

> 		  (props-plist (cadr elem))
> 		  (props-alist-strg-keys
> 		   (org-plist-to-alist props-plist nil t))

You shouldn't build this alist each time. More on this below.

> 		  (parser-keywords
> 		   (list :raw-value :title :alt-title :begin :end
> 			 :pre-blank :post-blank :contents-begin
> 			 :contents-end :level :priority :tags
> 			 :todo-keyword :todo-type :scheduled
> 			 :deadline :closed :archivedp :commentedp
> 			 :footnote-section-p))
> 		  (parser-keywords-but-special-props
> 		   (list :raw-value :title :alt-title :begin :end
> 			 :pre-blank :post-blank :contents-begin
> 			 :contents-end :level :archivedp
> 			 :commentedp :footnote-section-p))

This can be ignored.

> 		  ;; FIXME necessary?
> 		  ;; for backward compability only
> 		  (excluded
> 		   '("TODO" "TAGS" "ALLTAGS" "PRIORITY" "BLOCKED"))
> 		  sym)
> 	     (if specific-prop
> 		 (assoc specific-prop props-alist-strg-keys)

THEN branch is not right, as SPECIFIC-PROP could contain a regexp.

> 	       (let ((all-but-parser
> 		      (progn
> 			(setplist 'sym props-plist)
> 			(mapc (lambda (--kw)
> 				(remprop 'sym --kw))
> 			      parser-keywords-but-special-props)
> 			(org-plist-to-alist
> 			 (symbol-plist 'sym) nil t)))

Please ignore the parser part. Also, there's no reason to use a plist
associated to a symbol (i.e., `setplist').

BTW, the algorithm doesn't sound right. You are retrieving all values
from all properties, and then filter out those you do not need. It would
be simpler to list the properties you need and extract their value. No
need to build an alist or juggle with `org-plist-to-alist' then.

Therefore, I suggest to create an alist, as a defconst, e.g.,
`org-internal-properties-alist', that will contain associations between
an internal property name and its key

  '(("TODO" . :todo-keyword)
    ("TAGS" . :tags)
    ...)

When you want to extract a value from the element, you first check this
constant and, if that fails, you build the key out of the name:

 (or (cdr (assoc-ignore-case current-property org-internal-properties-alist))
     (intern (concat ":" current-property)))

> 		     (downcased-special-props
> 		      (mapcar 'downcase org-special-properties)))

Not useful, you can ignore it too: there are `member-ignore-case' and
`assoc-ignore-case'.

> 		 (case which

`case' keys need not be quoted:

  (case which
    (all all-but-parser)
    ...)

> 		   ('all all-but-parser)
> 		   ('non-org (remove-if
> 			      (lambda (--item)
> 				(member
> 				 (car --item)
> 				 (append
> 				  org-special-properties
> 				  org-default-properties
> 				  org-custom-properties
> 				  org-element-document-properties
> 				  org-global-properties
> 				  org-global-properties-fixed
> 				  org-file-properties)))
> 			      all-but-parser))
> 		   ;; FIXME necessary?
> 		   ;; for backward compability only
> 		   ('standard (remove-if
> 			       (lambda (--item)
> 				 (member (car --item) excluded))
> 				 all-but-parser))
> 		   ;; return plist
> 		   ('parser (setplist 'sym props-plist)
> 			    (mapc (lambda (--kw)
> 				    (remprop 'sym --kw))
> 				  (set-difference
> 				   (org-plist-keys props-plist t)
> 				   parser-keywords))
> 			    (symbol-plist 'sym))
> 		   ('special (remove-if-not
> 			      (lambda (--item)
> 				(member
> 				 (car --item)
> 				 downcased-special-props))
> 			      props-alist-strg-keys))
> 		   ('default (remove-if-not
> 			      (lambda (--item)
> 				(member (car --item)
> 					org-default-properties))
> 			      props-alist-strg-keys))
> 		   ('custom (remove-if-not
> 			     (lambda (--item)
> 			       (member (car --item)
> 				       org-custom-properties))
> 			     props-alist-strg-keys))
> 		   ('document (remove-if-not
> 			       (lambda (--item)
> 				 (member
> 				  (car --item)
> 				  org-element-document-properties))
> 			       props-alist-strg-keys))
> 		   ('global (remove-if-not
> 			     (lambda (--item)
> 			       (member (car --item)
> 				       org-global-properties))
> 			     props-alist-strg-keys))
> 		   ('global-fixed (remove-if-not
> 				   (lambda (--item)
> 				     (assoc
> 				      (car --item)
> 				      org-global-properties-fixed))
> 				   props-alist-strg-keys))
> 		   ('file (remove-if-not
> 			   (lambda (--item)
> 			     (member (car --item)
> 				     org-file-properties))
> 			   props-alist-strg-keys))
> 		   (t (when (consp which)
> 			(ignore-errors
> 			  (let ((rgxp (rx-to-string which)))
> 			    (remove-if-not
> 			     (lambda (--item)
> 			       (string-match rgxp (car --item)))
> 			     all-but-parser)))))))))))))))

Since you only need to extract values, you need not use `remove-if-not':

  (let ((predicate
         (case which
           (all #'identity)
           (special (lambda (p) (member-ignore-case p org-special-properties)))
           (default (lambda (p) (member p org-default-properties)))
           ...
           (otherwise (lambda (p) (org-string-match-p which p))))))
    (mapcar (lambda (p)
              (when (funcall predicate p)
                (cons p
                      (org-element-property
                       (or (cdr (assoc-ignore-case current-property
                                                   org-internal-properties-alist))
                           (intern (concat ":" current-property)))
                       elem))))
            list-of-all-properties))

> ** helper function (org-macs.el)

None of them is needed actually.

> ** tests (test-org.el)
>
>
> #+begin_src emacs-lisp
> ;;; Properties
>
> (defconst test-org/org-entry-properties-temp-text
> "* DONE [#A] headline <2014-07-31 Do> :tag:
>   DEADLINE: <2014-08-01 Fr 08:00>
>   - State \"DONE\"       from \"WAITING\"    [2014-07-31 Do 22:45]
>   - State \"WAITING\"    from \"TODO\"       [2014-07-31 Do 14:46] \\
>     testing
>   :PROPERTIES:
>   :CATEGORY: mycat
>   :VISIBILITY_ALL: folded children all
>   :foo-key1: val1
>   :foo-key2: val2
>   :ID:       3996b55d-d678-43a4-af1f-48ed22b5f414
>   :CUSTOM_ID: abc123
>   :bar:      loo
>   :END:
>   [2014-07-31 Do 14:45]
> "
>  "Headline used to test `org-entry-properties'.")
>
> (ert-deftest test-org/org-entry-properties-1 ()
>   "Test of `org-entry-properties' specifications."
>   (should
>   (equal '(("priority" . 65) ("tags" "tag") ("todo-keyword" . "DONE") ("todo-type" . done) ("deadline" timestamp (:type active :raw-value "<2014-08-01 Fr 08:00>" :year-start 2014 :month-start 8 :day-start 1 :hour-start 8 :minute-start 0 :year-end 2014 :month-end 8 :day-end 1 :hour-end 8 :minute-end 0 :begin 56 :end 77 :post-blank 0)) ("CATEGORY" . "mycat") ("VISIBILITY_ALL" . "folded children all") ("FOO-KEY1" . "val1") ("FOO-KEY2" . "val2") ("ID" . "3996b55d-d678-43a4-af1f-48ed22b5f414") ("CUSTOM_ID" . "abc123") ("BAR" . "loo"))
> 	  (org-test-with-temp-text
> 	      test-org/org-entry-properties-temp-text
> 	    (org-entry-properties nil nil nil)))))

I suggest to focus on minimal tests, since those are easier to debug.
IOW, do not load an unwieldy properties drawer but write the particular
part you want to test.


[...]

> (ert-deftest test-org/org-entry-properties-3 ()
>   "Test 3 of `org-entry-properties' specifications."
>   (should
>    (equal nil
> 	  (org-test-with-temp-text
> 	      test-org/org-entry-properties-temp-text
> 	    (org-entry-properties nil nil 'foo)))))

`should-not' is better than `should' + (equal nil ...)

Note that implementation doesn't handle CLOCK, CLOCKSUM, CLOCKSUM_T,
TIMESTAMP, TIMESTAMP_IA, and BLOCKED values yet.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-07-31 23:21 [RFC] Rewrite `org-entry-properties' using parser Thorsten Jolitz
  2014-08-01 11:44 ` Bastien
  2014-08-01 13:28 ` Nicolas Goaziou
@ 2014-08-01 18:41 ` Erik Hetzner
  2014-08-03 18:59   ` John Kitchin
  2 siblings, 1 reply; 13+ messages in thread
From: Erik Hetzner @ 2014-08-01 18:41 UTC (permalink / raw)
  To: emacs-orgmode

At Fri, 01 Aug 2014 01:21:47 +0200,
Thorsten Jolitz wrote:
> 
> Hi List,
> 
> here is my first take of rewriting `org-entry-properties'.
> 
> The existing function predates the new parser and some Org variables,
> and thus does the parsing and the property classification itself. The
> new version leaves parsing to the parser and property classification
> (mostly) to existing Org variables, resulting in much simpler code. 
>
> […]

Hi Thorsten,

This doesn’t directly related to this work, but I have been trying to
come up with a workflow for using org-mode for research and have had
trouble with the fact that user properties cannot be multi-valued. So
if a user wants to assign subjects to an entry, they need to figure
some way to assign multiple subjects.

There are workarounds, but it would be helpful if user properties
could be multivalued. I don’t know if this is feasible given the
current codebase; when I looked into it, it seemed pretty difficult.

best, Erik

-- 
Sent from my free software system <http://fsf.org/>.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-01 18:41 ` Erik Hetzner
@ 2014-08-03 18:59   ` John Kitchin
  2014-08-04  3:45     ` Erik Hetzner
  0 siblings, 1 reply; 13+ messages in thread
From: John Kitchin @ 2014-08-03 18:59 UTC (permalink / raw)
  To: Erik Hetzner; +Cc: emacs-orgmode

I have used the following approaches in the past:

Lisp lists, and use read later to get them.
* Some heading
  :PROPERTIES:
  :SUBJECT:  '(subject1 subject2 subject3)
  :END:

#+BEGIN_SRC emacs-lisp 
(read (org-entry-get (point) "SUBJECT"))
#+END_SRC

#+RESULTS:
| quote | (subject1 subject2 subject3) |


* Second heading 
  :PROPERTIES:
  :SUBJECT:  subject1 subject2 subject3
  :END:

delimited strings. You have to split them them yourself later if you
are using the properties in code. You can delimit on spaces, commas, etc... depending on your subjects.

#+BEGIN_SRC emacs-lisp
(split-string (org-entry-get (point) "SUBJECT"))
#+END_SRC

#+RESULTS:
| subject1 | subject2 | subject3 |

They are both pretty flexible.

Erik Hetzner <egh@e6h.org> writes:

> At Fri, 01 Aug 2014 01:21:47 +0200,
> Thorsten Jolitz wrote:
>> 
>> Hi List,
>> 
>> here is my first take of rewriting `org-entry-properties'.
>> 
>> The existing function predates the new parser and some Org variables,
>> and thus does the parsing and the property classification itself. The
>> new version leaves parsing to the parser and property classification
>> (mostly) to existing Org variables, resulting in much simpler code. 
>>
>> […]
>
> Hi Thorsten,
>
> This doesn’t directly related to this work, but I have been trying to
> come up with a workflow for using org-mode for research and have had
> trouble with the fact that user properties cannot be multi-valued. So
> if a user wants to assign subjects to an entry, they need to figure
> some way to assign multiple subjects.
>
> There are workarounds, but it would be helpful if user properties
> could be multivalued. I don’t know if this is feasible given the
> current codebase; when I looked into it, it seemed pretty difficult.
>
> best, Erik

-- 
-----------------------------------
John Kitchin
Professor
Doherty Hall A207F
Department of Chemical Engineering
Carnegie Mellon University
Pittsburgh, PA 15213
412-268-7803
http://kitchingroup.cheme.cmu.edu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-03 18:59   ` John Kitchin
@ 2014-08-04  3:45     ` Erik Hetzner
  2014-08-04  4:07       ` Aaron Ecay
  0 siblings, 1 reply; 13+ messages in thread
From: Erik Hetzner @ 2014-08-04  3:45 UTC (permalink / raw)
  To: John Kitchin; +Cc: emacs-orgmode

Hi John,

Thanks for the tips! I have been using this:

* Foo
  :PROPERTIES:
  :subject:  Bar; Baz
  :END:

This allows me to search with subject={Bar}, which is very helpful.
But it would be nice if org supported multi-valued properties. Again,
though, I don’t know how hard this would be.

best, Erik

At Sun, 03 Aug 2014 14:59:02 -0400,
John Kitchin wrote:
> 
> I have used the following approaches in the past:
> 
> Lisp lists, and use read later to get them.
> * Some heading
>   :PROPERTIES:
>   :SUBJECT:  '(subject1 subject2 subject3)
>   :END:
> 
> #+BEGIN_SRC emacs-lisp 
> (read (org-entry-get (point) "SUBJECT"))
> #+END_SRC
> 
> #+RESULTS:
> | quote | (subject1 subject2 subject3) |
> 
> 
> * Second heading 
>   :PROPERTIES:
>   :SUBJECT:  subject1 subject2 subject3
>   :END:
> 
> delimited strings. You have to split them them yourself later if you
> are using the properties in code. You can delimit on spaces, commas, etc... depending on your subjects.
> 
> #+BEGIN_SRC emacs-lisp
> (split-string (org-entry-get (point) "SUBJECT"))
> #+END_SRC
> 
> #+RESULTS:
> | subject1 | subject2 | subject3 |
> 
> They are both pretty flexible.

> 
> Erik Hetzner <egh@e6h.org> writes:
> 
> > At Fri, 01 Aug 2014 01:21:47 +0200,
> > Thorsten Jolitz wrote:
> >> 
> >> Hi List,
> >> 
> >> here is my first take of rewriting `org-entry-properties'.
> >> 
> >> The existing function predates the new parser and some Org variables,
> >> and thus does the parsing and the property classification itself. The
> >> new version leaves parsing to the parser and property classification
> >> (mostly) to existing Org variables, resulting in much simpler code. 
> >>
> >> […]
> >
> > Hi Thorsten,
> >
> > This doesn’t directly related to this work, but I have been trying to
> > come up with a workflow for using org-mode for research and have had
> > trouble with the fact that user properties cannot be multi-valued. So
> > if a user wants to assign subjects to an entry, they need to figure
> > some way to assign multiple subjects.
> >
> > There are workarounds, but it would be helpful if user properties
> > could be multivalued. I don’t know if this is feasible given the
> > current codebase; when I looked into it, it seemed pretty difficult.
> >
> > best, Erik
> 
> -- 
> -----------------------------------
> John Kitchin
> Professor
> Doherty Hall A207F
> Department of Chemical Engineering
> Carnegie Mellon University
> Pittsburgh, PA 15213
> 412-268-7803
> http://kitchingroup.cheme.cmu.edu

-- 
Sent from my free software system <http://fsf.org/>.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-04  3:45     ` Erik Hetzner
@ 2014-08-04  4:07       ` Aaron Ecay
  2014-08-04 14:11         ` Erik Hetzner
  0 siblings, 1 reply; 13+ messages in thread
From: Aaron Ecay @ 2014-08-04  4:07 UTC (permalink / raw)
  To: Erik Hetzner, John Kitchin; +Cc: emacs-orgmode

Hi Erik,

2014ko abuztuak 3an, Erik Hetzner-ek idatzi zuen:
> 
> Hi John,
> 
> Thanks for the tips! I have been using this:
> 
> * Foo
>   :PROPERTIES:
>   :subject:  Bar; Baz
>   :END:
> 
> This allows me to search with subject={Bar}, which is very helpful.
> But it would be nice if org supported multi-valued properties. 

It does, at least to a first approximation.  Look at the functions
described in:

(info "(org) Using the property API")

I’m not sure how well this feature is integrated with agenda
search/filtering, though – maybe you should give it a try.

-- 
Aaron Ecay

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-04  4:07       ` Aaron Ecay
@ 2014-08-04 14:11         ` Erik Hetzner
  0 siblings, 0 replies; 13+ messages in thread
From: Erik Hetzner @ 2014-08-04 14:11 UTC (permalink / raw)
  To: Aaron Ecay, emacs-orgmode

At Mon, 04 Aug 2014 00:07:46 -0400,
Aaron Ecay wrote:
> 
> Hi Erik,
> 
> 2014ko abuztuak 3an, Erik Hetzner-ek idatzi zuen:
> > 
> > […]
> >
> > This allows me to search with subject={Bar}, which is very helpful.
> > But it would be nice if org supported multi-valued properties. 
> 
> It does, at least to a first approximation.  Look at the functions
> described in:
> 
> (info "(org) Using the property API")
> 
> I’m not sure how well this feature is integrated with agenda
> search/filtering, though – maybe you should give it a try.

Hi Aaron,

Unfortunately those functions (org-entry-get-multivalued-property,
etc.) are completely isolated from all other property related code, so
search and filtering does not recognize them. Furthermore, they simply
split properties string on whitespace.

best, Erik

-- 
Sent from my free software system <http://fsf.org/>.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-01 13:28 ` Nicolas Goaziou
@ 2014-08-05 12:52   ` Thorsten Jolitz
  2014-08-06  9:59     ` Rasmus
  2014-08-06 12:04     ` Nicolas Goaziou
  0 siblings, 2 replies; 13+ messages in thread
From: Thorsten Jolitz @ 2014-08-05 12:52 UTC (permalink / raw)
  To: emacs-orgmode

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Thorsten Jolitz <tjolitz@gmail.com> writes:
>
>> here is my first take of rewriting `org-entry-properties'.
>
> Interesting. A first round of comments follows.

Thanks for the review and the comments. But this looks like an entirely
different function to me, and IMO it does not make much sense that you
explain what you want and I type it in (because instead of explaining
you could just as well type it yourself in the same time), so I rather
leave this to you (or whoever wants to do it).

-- 
cheers,
Thorsten

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-05 12:52   ` Thorsten Jolitz
@ 2014-08-06  9:59     ` Rasmus
  2014-08-06 12:04     ` Nicolas Goaziou
  1 sibling, 0 replies; 13+ messages in thread
From: Rasmus @ 2014-08-06  9:59 UTC (permalink / raw)
  To: emacs-orgmode

Hi,

Thorsten Jolitz <tjolitz@gmail.com> writes:

> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
>
>> Thorsten Jolitz <tjolitz@gmail.com> writes:
>>
>>> here is my first take of rewriting `org-entry-properties'.
>>
>> Interesting. A first round of comments follows.
> [...]
> But this looks like an entirely different function to me, and IMO it
> does not make much sense that you explain what you want and I type
> it in (*because instead of explaining you could just as well type it
> yourself in the same time*), so I rather leave this to you (or
> whoever wants to do it).

[my emphasis added]

This is not directly related, I'm just "guarding" my own interests
here.

I *really* appreciate Nicolas' way of giving comments as I learn a lot
about my mistakes this way and how not to repeat them (though I most
likely do).  Surely Nicolas could take most of the ox-related patches
and iron out bugs and "bugs" the first time they are submitted.
Writing emails proposing change takes more time.  It's meant as a
favor and frankly the alternative is less pleasing — at least for me.

Granted, you, Thorsten, is a lot more "senior" is writing code than me
and perhaps you see it differently.

Cheers,
Rasmus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-05 12:52   ` Thorsten Jolitz
  2014-08-06  9:59     ` Rasmus
@ 2014-08-06 12:04     ` Nicolas Goaziou
  2014-08-21  0:16       ` Thorsten Jolitz
  1 sibling, 1 reply; 13+ messages in thread
From: Nicolas Goaziou @ 2014-08-06 12:04 UTC (permalink / raw)
  To: Thorsten Jolitz; +Cc: emacs-orgmode

Hello,

Thorsten Jolitz <tjolitz@gmail.com> writes:

> Thanks for the review and the comments. But this looks like an entirely
> different function to me

I think you're confused between `org-element-at-point' and
`org-entry-properties'. As I pointed out, there are properties that
ought to stay specific to the former (e.g. :contents-begin) and some
specific to the latter (e.g. :clocksum).

As I said, sharing code for the common parts is "interesting". But it is
not sufficient.

Another option would be to discuss if `org-entry-properties' is needed
at all. AFAICT by grepping for "org-entry-properties" through the code
base, besides "org-pcomplete.el", no call is really needed. Most
functions really need a single property. It is inefficient to grab them
all just to extract one.

> and IMO it does not make much sense that you explain what you want
> and I type it in (because instead of explaining you could just as well
> type it yourself in the same time)

You may want to read my answer again. There is no "write this, write
that" in it. I'm pointing out what is missing or flawed and _suggesting_
alternative approaches.

> so I rather leave this to you (or whoever wants to do it).

Unfortunately, my plate is full at the moment.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC] Rewrite `org-entry-properties' using parser
  2014-08-06 12:04     ` Nicolas Goaziou
@ 2014-08-21  0:16       ` Thorsten Jolitz
  0 siblings, 0 replies; 13+ messages in thread
From: Thorsten Jolitz @ 2014-08-21  0:16 UTC (permalink / raw)
  To: emacs-orgmode

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

Hello,

> You may want to read my answer again. There is no "write this, write
> that" in it. I'm pointing out what is missing or flawed and _suggesting_
> alternative approaches.

I appreciate your comments and input, but unfortunately I gave up on the
original idea because I did not want to make another "much more time
consuming than planned" side-project out of this.

>> so I rather leave this to you (or whoever wants to do it).

> Unfortunately, my plate is full at the moment.

Ok, so I simplified the task and wrote 

,----[ C-h f org-dp-filter-node-props RET ]
| org-dp-filter-node-props is a Lisp function in `org-dp-lib.el'.
| 
| (org-dp-filter-node-props FILTER &optional NEGATE-P SILENT-P)
| 
| Return filtered node-properties.
| FILTER should either be the car of one of the cons-pairs in
| `org-dp-prop-filters' or a regexp-string. If NEGATE-P is non-nil,
| the properties not matched by the filter are returned. If
| SILENT-P is non-nil, no message is printed if no property-drawer
| is found.
`----

 - this function acts on element-type 'node-property', not 'headline'

 - it does not attempt to replace `org-entry-properties' or to be
   compatible with its signature, its written from scratch.

but it does what I want - make it easy to filter out application
properties and user defined properties from the usual property-mix found
in property drawers.

It relies on the following constants:

,----[ C-h v org-dp-prop-filters RET ]
| org-dp-prop-filters is a variable defined in `org-dp-lib.el'.
| Its value is ((special . org-special-properties)
|  (custom . org-custom-properties)
|  (default . org-default-properties)
|  (file . org-file-properties)
|  (global mapcar 'car org-global-properties)
|  (org)
|  (user-defined))
| 
| Alist of filter-types and associated property-classes.
`----

(do I miss an Org property class here?)

,----[ C-h v org-dp-org-props RET ]
| org-dp-org-props is a variable defined in `org-dp-lib.el'.
| [...]
| Union of special, custom, file and global properties, as well
|   as those properties specified by the user in customizable
|   variable `org-dp-misc-props'.
`----

Thus given the following Org headline two typical function calls would
be:

#+BEGIN_SRC emacs-lisp :results raw
(save-excursion
  (outline-next-heading)
  (org-dp-filter-node-props "^foo-."))
#+END_SRC

#+results:
((foo-star . star1) (foo-bar . bar1))

#+BEGIN_SRC emacs-lisp :results raw
(save-excursion
  (outline-next-heading)
  (org-dp-filter-node-props 'org t))
#+END_SRC

#+results:
((cho-bar . bar2) (foo-star . star1) (foo-bar . bar1))

* ORG SCRATCH
  :PROPERTIES:
  :CUSTOM_ID: abc123
  :CATEGORY: mycat
  :foo-bar:  bar1
  :foo-star: star1
  :cho-bar:  bar2
  :END:

PS

For completeness I attach the code:

#+BEGIN_SRC emacs-lisp
(defun org-dp-filter-node-props (filter &optional negate-p silent-p)
  "Return filtered node-properties.
FILTER should either be the car of one of the cons-pairs in
`org-dp-prop-filters' or a regexp-string. If NEGATE-P is non-nil,
the properties not matched by the filter are returned. If
SILENT-P is non-nil, no message is printed if no property-drawer
is found."
  (let ((props (save-excursion
		 (and
		  (or (org-at-heading-p)
		      (outline-previous-heading))
		  (re-search-forward org-property-drawer-re
				     nil 'NOERROR 1)
		  (progn
		    (goto-char (match-beginning 0))
		    (org-dp-contents)))))
	filtered-props)
    (if (not props)
	(unless silent-p
	  (message
	   "Could not find properties at point %d in buffer %s."
	   (point) (current-buffer)))
      (org-element-map props 'node-property
	(lambda (--prop)
	  (let* ((key (org-element-property :key --prop))
		 (val (org-element-property :value --prop))
		 (memberp (case filter
			    ((special custom default file global)
			     (member-ignore-case
			      key
			      (eval
			       (cdr-safe
				(assoc filter
				       org-dp-prop-filters)))))
			    (org (member-ignore-case
				  key org-dp-org-props))
			    (t (if (stringp filter)
				   (string-match filter key)
				 (error "Not a valid filter: %s"
					filter))))))
	    (when (or (and negate-p (not memberp))
		      (and (not negate-p) memberp))
	      (setq filtered-props
		    (cons (cons key val) filtered-props))))))
      filtered-props)))
#+END_SRC

-- 
cheers,
Thorsten

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-08-21  0:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-31 23:21 [RFC] Rewrite `org-entry-properties' using parser Thorsten Jolitz
2014-08-01 11:44 ` Bastien
2014-08-01 12:44   ` Thorsten Jolitz
2014-08-01 13:28 ` Nicolas Goaziou
2014-08-05 12:52   ` Thorsten Jolitz
2014-08-06  9:59     ` Rasmus
2014-08-06 12:04     ` Nicolas Goaziou
2014-08-21  0:16       ` Thorsten Jolitz
2014-08-01 18:41 ` Erik Hetzner
2014-08-03 18:59   ` John Kitchin
2014-08-04  3:45     ` Erik Hetzner
2014-08-04  4:07       ` Aaron Ecay
2014-08-04 14:11         ` Erik Hetzner

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).