Ihor Radchenko <yanta...@gmail.com> writes:
> Note that your patch is >15LOC long and you need to sign the copyright
> agreement with FSF in order to contribute. See
> https://orgmode.org/worg/org-contribute.html#copyright
I've already submitted a copyright assignment to the FSF in order to
publish on ELPA. Do I need one specific to org-mode?
> Some comments on the patch:
>
>> * lisp/org.el (org-property-separators, org-property-get-separator):
>> Created.
>
> I'd make the function private: org--property-get-separator. It is not
> intended as an API function.
I agree, that was an oversight.
>
>> (org-entry-get, org-entry-get-with-inheritance): Use new
>> org-property-get-separator function.
>
>> org-property-separators is a customization option that allows for
>
> Please quote the function name as `org-property-get-separator'.
>
No problem.
>> +If a property is specified multiple times with a =+=, like
>> +=:EXPORT_FILE_NAME+:=, the old behavior was to always combine them
>> +with a single space. For the new variable, the car of each item in
>> the
>
> Please, use double space " " to separate sentences. Also, see
> doc/Documentation_Standards.org
No problem.
>
>> +For example, in order to combine =EXPORT_FILE_NAME= properties with a
>> +forward slash =/=, one can use
>> +
>> +#+begin_src emacs-lisp
>> +(setq org-use-property-inheritance '("EXPORT_FILE_NAME")
>> + org-property-separators '((("EXPORT_FILE_NAME") . "/")))
>> +#+end_src
>
> This example is a bit confusing because it is unclear what you want to
> achieve and why you also need to set inheritance.
Inheritance is the most likely scenario one would need to use the '+'
property syntax, but I do agree it's kinda distracting and not
absolutely necessary in order to get the correct behavior.
>
>> +(defcustom org-property-separators nil
>> ...
>> + :group 'org-properties
>> + :type '(alist :key-type string :value-type sexp))
>
> This defcustom type does not match what you described in the docstring.
> You need something like :type '(alist :key-type (choice string (repeat
> string)) :value-type string)
>
> Best,
> Ihor
Setting ':value-type string' is confusing, in my opinion, because the
default single space looks like: in the customization buffer, which is
indistinguishable from no space: . I just found out about the restricted-sexp
type, which I think makes the customization buffer more user-friendly.
>From f474cb25840fdc6b24618b1452cb7fdd32545092 Mon Sep 17 00:00:00 2001
From: Tyler Grinn <tylergr...@gmail.com>
Date: Mon, 9 May 2022 15:52:58 -0400
Subject: [PATCH] lisp/org.el: Add org-property-separators option
* lisp/org.el (org-property-separators, org-property-get-separator):
Created.
(org-entry-get, org-entry-get-with-inheritance): Use new
`org-property-get-separator' function.
* testing/lisp/test-org.el (test-org/entry-get): Added tests for
combining properties with custom separators
`org-property-separators' is a customization option that allows for
properties to be combined using a separator other than the default (a
single space). It is an alist with the car of each element being a
list of property names or regular expression and the cdr being the
separator string, like '((("EXPORT_FILE_NAME") . "/")).
---
etc/ORG-NEWS | 31 ++++++++++++++++++++++++++
lisp/org.el | 47 +++++++++++++++++++++++++++++++++-------
testing/lisp/test-org.el | 30 ++++++++++++++++++++++++-
3 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 27de6da62..9d1d2cdcf 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -141,6 +141,37 @@ discouraged when working with Org files.
** New features
+*** New customization option =org-property-separators=
+A new alist variable to control how properties are combined.
+
+If a property is specified multiple times with a =+=, like
+
+#+begin_src org
+:PROPERTIES:
+:EXPORT_FILE_NAME: some/path
+:EXPORT_FILE_NAME+: to/file
+:END:
+#+end_src
+
+the old behavior was to always combine them with a single space
+(=some/path to/file=). For the new variable, the car of each item in
+the alist should be either a list of property names or a regular
+expression, while the cdr should be the separator to use when
+combining that property.
+
+The default value for the separator is a single space, if none of the
+provided items in the alist match a given property.
+
+For example, in order to combine =EXPORT_FILE_NAME= properties with a
+forward slash =/=, one can use
+
+#+begin_src emacs-lisp
+(setq org-property-separators '((("EXPORT_FILE_NAME") . "/")))
+#+end_src
+
+The example above would then produce the property value
+=some/path/to/file=.
+
*** New library =org-persist.el= implements variable persistence across Emacs sessions
The library stores variable data in ~org-persist-directory~ (set to XDG
diff --git a/lisp/org.el b/lisp/org.el
index cab59b87c..643fd6b73 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2850,6 +2850,34 @@ in this variable)."
(member-ignore-case property org-use-property-inheritance))
(t (error "Invalid setting of `org-use-property-inheritance'"))))
+(defcustom org-property-separators nil
+ "An alist to control how properties are combined.
+
+The car of each item should be either a list of property names or
+a regular expression, while the cdr should be the separator to
+use when combining that property.
+
+If an alist item cannot be found that matches a given property, a
+single space will be used as the separator."
+ :group 'org-properties
+ :type '(alist :key-type (choice (repeat :tag "Properties" string)
+ (string :tag "Regular Expression"))
+ :value-type (restricted-sexp :tag "Separator"
+ :match-alternatives (stringp)
+ :value " ")))
+
+(defun org--property-get-separator (property)
+ "Get the separator to use for combining PROPERTY."
+ (or
+ (catch 'separator
+ (dolist (spec org-property-separators)
+ (if (listp (car spec))
+ (if (member property (car spec))
+ (throw 'separator (cdr spec)))
+ (if (string-match-p (car spec) property)
+ (throw 'separator (cdr spec))))))
+ " "))
+
(defcustom org-columns-default-format "%25ITEM %TODO %3PRIORITY %TAGS"
"The default column format, if no other format has been defined.
This variable can be set on the per-file basis by inserting a line
@@ -12355,7 +12383,9 @@ value higher up the hierarchy."
(org-entry-get-with-inheritance property literal-nil))
(t
(let* ((local (org--property-local-values property literal-nil))
- (value (and local (mapconcat #'identity (delq nil local) " "))))
+ (value (and local (mapconcat #'identity
+ (delq nil local)
+ (org--property-get-separator property)))))
(if literal-nil value (org-not-nil value)))))))
(defun org-property-or-variable-value (var &optional inherit)
@@ -12464,7 +12494,8 @@ However, if LITERAL-NIL is set, return the string value \"nil\" instead."
(catch 'exit
(let ((element (or element
(and (org-element--cache-active-p)
- (org-element-at-point nil 'cached)))))
+ (org-element-at-point nil 'cached))))
+ (separator (org--property-get-separator property)))
(if element
(let ((element (org-element-lineage element '(headline org-data inlinetask) 'with-self)))
(while t
@@ -12472,8 +12503,8 @@ However, if LITERAL-NIL is set, return the string value \"nil\" instead."
(v (if (listp v) v (list v))))
(when v
(setq value
- (concat (mapconcat #'identity (delq nil v) " ")
- (and value " ")
+ (concat (mapconcat #'identity (delq nil v) separator)
+ (and value separator)
value)))
(cond
((car v)
@@ -12484,15 +12515,15 @@ However, if LITERAL-NIL is set, return the string value \"nil\" instead."
(t
(let ((global (org--property-global-or-keyword-value property literal-nil)))
(cond ((not global))
- (value (setq value (concat global " " value)))
+ (value (setq value (concat global separator value)))
(t (setq value global))))
(throw 'exit nil))))))
(while t
(let ((v (org--property-local-values property literal-nil)))
(when v
(setq value
- (concat (mapconcat #'identity (delq nil v) " ")
- (and value " ")
+ (concat (mapconcat #'identity (delq nil v) separator)
+ (and value separator)
value)))
(cond
((car v)
@@ -12513,7 +12544,7 @@ However, if LITERAL-NIL is set, return the string value \"nil\" instead."
(t
(let ((global (org--property-global-or-keyword-value property literal-nil)))
(cond ((not global))
- (value (setq value (concat global " " value)))
+ (value (setq value (concat global separator value)))
(t (setq value global))))
(throw 'exit nil))))))))
(if literal-nil value (org-not-nil value)))))
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 0cee31d4e..c0c13f586 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -5997,7 +5997,35 @@ Paragraph<point>"
(org-test-with-temp-text
":PROPERTIES:\n:A: 0\n:END:\n#+PROPERTY: A 1\n* H\n:PROPERTIES:\n:A+: 2\n:END:"
(org-mode-restart)
- (org-entry-get (point-max) "A" t)))))
+ (org-entry-get (point-max) "A" t))))
+ ;; Use alternate separators
+ (should
+ (equal "0~2"
+ (org-test-with-temp-text
+ ":PROPERTIES:\n:A: 0\n:A+: 2\n:END:"
+ (let ((org-property-separators '((("A") . "~"))))
+ (org-entry-get (point) "A")))))
+ ;; Default separator is single space
+ (should
+ (equal "0 2"
+ (org-test-with-temp-text
+ ":PROPERTIES:\n:A: 0\n:B: 1\n:A+: 2\n:B+: 3\n:END:"
+ (let ((org-property-separators '((("B") . "~"))))
+ (org-entry-get (point) "A")))))
+ ;; Regular expression matching for separator
+ (should
+ (equal "0/2"
+ (org-test-with-temp-text
+ ":PROPERTIES:\n:A: 0\n:A+: 2\n:END:"
+ (let ((org-property-separators '((("B") . "~") ("[AC]" . "/"))))
+ (org-entry-get (point) "A")))))
+ ;; Separator works with inheritance
+ (should
+ (equal "1~2"
+ (org-test-with-temp-text
+ "* H\n:PROPERTIES:\n:A: 1\n:END:\n** H2\n:PROPERTIES:\n:A+: 2\n:END:"
+ (let ((org-property-separators '((("A") . "~"))))
+ (org-entry-get (point-max) "A" t))))))
(ert-deftest test-org/entry-properties ()
"Test `org-entry-properties' specifications."
--
2.35.3
