On 14/11/2022 10:29, Ihor Radchenko wrote:
I went through the patch and tried to clarify the wording.
Especially in the defcustom docstring.
I do not mind in general.
Please, remove a stray space in the defcustom.
I also added the dumb fallback to the default value.
I feel that otherwise the description of too confusing.
I am unsure concerning adding it to the default value. From my point of
view, it is better to ask user to clarify their intention. I believe
that the obscure error message is the real bug, not that Org can not
handle too short ID by default.
I think, for new users default value should include just strict variants
of `org-attach-id-uuid-folder-format' and
`org-attach-id-ts-folder-format' that checks ID format as in the example
in ORG-NEWS. Changing of `org-id-method' will not cause non-optimal
subdir layout for the same value of 'org-attach-id-to-path-function-list'.
It will cause a problem for users who changed `org-id-method' in the
past, so they have either XX/timestamp or YYYYMM/uuid subdirectories. It
may be solved by adding original variants of
`org-attach-id-uuid-folder-format' and `org-attach-id-ts-folder-format',
but perhaps it is better to add predicates for "wrong" style (UUID or
Org for timestamp and vice versa) just for backward compatibility.
It should be responsibility of the user to setup subdirs layout for
non-standard ID format. The dumb fallback is intended as an example and
a variant for those who do not have a lot of attachments and do not care
where they are stored.
+#+begin_src emacs-lisp
+(setq org-attach-id-to-path-function-list
+ '(;; When ID looks like an UUIDs or Org internal ID, use
+ ;; `org-attach-id-uuid-folder-format'.
+ (lambda (id)
+ (and (or (org-uuidgen-p id)
+ (string-match-p "[0-9a-z]\\{12\\}" id))
+ (org-attach-id-uuid-folder-format id)))
+ ;; When ID looks like a timestap-based ID. Group by year-month
+ ;; folders.
+ (lambda (id)
+ (and (string-match-p "[0-9]\\{8\\}T[0-9]\\{6\\}\.[0-9]\\{6\\}" id)
+ (org-attach-id-ts-folder-format id)))
+ ;; Any other ID goes into "important" folder.
+ (lambda (id) (format "important/%s/%s" (substring id 0 1) id))))
`org-attach-id-uuid-folder-format' and `org-attach-id-ts-folder-format'
here were added for users changed `org-id-method' in the past and so
having mixed subdirs layout with UUIDs in 6 character prefix directories
or timestamps in two characters folders.
+#+end_src
+(defun org-attach-id-fallback-folder-format (id)
+ "Return \"__/X/ID\" folder path as a dumb fallback.
+X is the first character in the ID string.
+
+This function may be appended to `org-attach-id-path-function-list' to
+provide a fallback for non-standard ID values that other functions in
+`org-attach-id-path-function-list' are unable to handle. For example,
+when the ID is too short for `org-attach-id-ts-folder-format'.
+
+However, we recommend to define a more specific function spreading
+entries over multiple folders. This function may create a large
+number of entries in a single folder, which may cause issues on some
+systems."
+ (format "__/%s/%s" (substring id 0 1) id))
Additional single character subdir level should be a minor improvement,
unless `org-id-prefix' is non-nil.
+(defcustom org-attach-id-to-path-function-list '(
org-attach-id-uuid-folder-format
space ----------------------------------------------^
+ org-attach-id-ts-folder-format
If strict variants of functions were used above then non-standard IDs
would be isolated in the directory returned by the next entry
+ org-attach-id-fallback-folder-format)
So I do not have strong objections since default value of
`org-attach-id-to-path-function-list' may be adjusted later. Maybe
strict variants of ID to subdir functions should be added as named
functions. Fortunately it does not conflict with the patch variant your
suggested. Feel free to commit your version.
