On 08/11/2022 12:08, Ihor Radchenko wrote:
I feel like this makes the docstring confusing.
Note that `org-attach-id-to-path-function-list':
I have tried to update the docstrings.
if: No attachment directory is associated with the current node, adjust
‘org-attach-id-to-path-function-list’
I do not think this message is unhelpful.
With your patch, such message will be displayed even when
`org-attach-preferred-new-method' is set to something other than 'id.
And the message will be wrong then.
I have moved `error' call despite I have not figured out how to trigger
the error for other options.
+(defun org-attach-id-fallback-folder-format (id)
+ "May be added last to `org-attach-id-path-function-list'.
This is not a proper first line in a function docstring. First line must
describe what the function does.
I am still unsure that in this case effect is more important than
purpose. The function is too specific.
P.S. At first I believed that you have some objections concerning
changed role of the first function in the list, not just how it is
documented.
From dfc35f46f59d5938f1a520b860c3eda36f49a9d5 Mon Sep 17 00:00:00 2001
From: Max Nikulin <maniku...@gmail.com>
Date: Mon, 7 Nov 2022 23:48:02 +0700
Subject: [PATCH v2] org-attach.el: ID to path functions may return nil
* lisp/org-attach.el (org-attach-dir-from-id): Ignore nil values
returned by entries from `org-attach-id-to-path-function-list'.
(org-attach-dir-get-create): Update error message to suggest
customization of `org-attach-id-to-path-function-list'.
(org-attach-id-to-path-function-list): Add to the docstring examples
how to handle unusual IDs.
(org-attach-id-uuid-folder-format, org-attach-id-ts-folder-format):
Return nil if ID is too short.
(org-attach-id-fallback-folder-format): New function that may be added
as the last element of `org-attach-id-path-function-list' to handle
unexpectedly short IDs.
Earlier an obscure error like 'org-attach-id-ts-folder-format: Args out
of range: "ftt", 0, 6' was signalled in the case of unexpectedly short
ID.
Reported by Janek F <xer...@pm.me>
https://list.orgmode.org/KC8PcypJapBpJQtJxM0kX5N7Z0THL2Lq6EQjBMzpw1-vgQf72egZ2JOIlTbPYiqAVD4MdSBhrhBZr2Ykf5DN1mocm1ANvvuKKZShlkgzKYM=@pm.me
---
lisp/org-attach.el | 98 ++++++++++++++++++++++++++++++----------------
1 file changed, 65 insertions(+), 33 deletions(-)
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index ef183474e..bd3034992 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -166,26 +166,56 @@ When set to `query', ask the user instead."
"Translate an UUID ID into a folder-path.
Default format for how Org translates ID properties to a path for
attachments. Useful if ID is generated with UUID."
- (format "%s/%s"
- (substring id 0 2)
- (substring id 2)))
+ (and (< 2 (length id))
+ (format "%s/%s"
+ (substring id 0 2)
+ (substring id 2))))
(defun org-attach-id-ts-folder-format (id)
"Translate an ID based on a timestamp to a folder-path.
Useful way of translation if ID is generated based on ISO8601
timestamp. Splits the attachment folder hierarchy into
year-month, the rest."
- (format "%s/%s"
- (substring id 0 6)
- (substring id 6)))
+ (and (< 6 (length id))
+ (format "%s/%s"
+ (substring id 0 6)
+ (substring id 6))))
+
+(defun org-attach-id-fallback-folder-format (id)
+ "Use \"__/ID\" folder path as a dumb fallback.
+May be added as the last element of `org-attach-id-path-function-list'
+to avoid errors for customized ID values that other functions
+are unable to handle. An example is too short ID for
+`org-attach-id-ts-folder-format'. However it is better to define
+a more specific function spreading entries over subfolders."
+ (format "__/%s" id))
(defcustom org-attach-id-to-path-function-list '(org-attach-id-uuid-folder-format
org-attach-id-ts-folder-format)
- "List of functions parsing an ID string into a folder-path.
-The first function in this list defines the preferred function
-which will be used when creating new attachment folders. All
-functions of this list will be tried when looking for existing
-attachment folders based on ID."
+ "List of functions tried to get a folder path for an ID string.
+Functions are called in order till some of them returns an existing
+folder. If no folder has been created yet for the given ID
+then first value defines folder name that will be created
+to store an attachment. Nil values are skipped.
+
+Preferred storage policy depends on `org-id-method' and should be chosen
+to avoid excessive number of entries in a single directory. List
+of functions allows to handle the case of mixed ID styles after
+change of ID generator.
+
+If format of some ID strings differs from `org-id-method' you may
+add `org-attach-id-fallback-folder-format' to the end of the list.
+A better option is to keep attachments inside a dedicated folder.
+A function like the following one may be added as first element
+of the list.
+
+ (defun my/attach-id-custom-folder-format (id)
+ (let ((case-fold-search t))
+ (unless
+ (or (org-uuidgen-p id)
+ (string-match-p \"[0-9a-z]\\\\=\\{12\\\\}\" id)
+ (string-match-p \"[0-9]\\\\=\\{8\\\\}T[0-9]\\\\=\\{6\\\\}\\.[0-9]\\\\=\\{6\\\\}\" id))
+ (format \"important/%s/%s\" (substring id 0 1) id))))"
:group 'org-attach
:package-version '(Org . "9.3")
:type '(repeat (function :tag "Function with ID as input")))
@@ -360,7 +390,7 @@ If no attachment directory can be derived, return nil."
(org-attach-check-absolute-path attach-dir))
((setq id (org-entry-get nil "ID" org-attach-use-inheritance))
(org-attach-check-absolute-path nil)
- (setq attach-dir (org-attach-dir-from-id id 'try-all))))
+ (setq attach-dir (org-attach-dir-from-id id 'existing))))
(if no-fs-check
attach-dir
(when (and attach-dir (file-directory-p attach-dir))
@@ -381,7 +411,11 @@ If the attachment by some reason cannot be created an error will be raised."
(setq answer (read-char-exclusive)))
(cond
((or (eq org-attach-preferred-new-method 'id) (eq answer ?1))
- (setq attach-dir (org-attach-dir-from-id (org-id-get nil t))))
+ (let ((id (org-id-get nil t)))
+ (or (setq attach-dir (org-attach-dir-from-id id))
+ (error "Failed to get folder for id %s, \
+adjust `org-attach-id-to-path-function-list'"
+ id))))
((or (eq org-attach-preferred-new-method 'dir) (eq answer ?2))
(setq attach-dir (org-attach-set-directory)))
((eq org-attach-preferred-new-method 'nil)
@@ -392,27 +426,25 @@ If the attachment by some reason cannot be created an error will be raised."
(make-directory attach-dir t))
attach-dir))
-(defun org-attach-dir-from-id (id &optional try-all)
+(defun org-attach-dir-from-id (id &optional existing)
"Return a folder path based on `org-attach-id-dir' and ID.
-If TRY-ALL is non-nil, try all id-to-path functions in
-`org-attach-id-to-path-function-list' and return the first path
-that exist in the filesystem, or the first one if none exist.
-Otherwise only use the first function in that list."
- (let ((attach-dir-preferred (expand-file-name
- (funcall (car org-attach-id-to-path-function-list) id)
- (expand-file-name org-attach-id-dir))))
- (if try-all
- (let ((attach-dir attach-dir-preferred)
- (fun-list (cdr org-attach-id-to-path-function-list)))
- (while (and fun-list (not (file-directory-p attach-dir)))
- (setq attach-dir (expand-file-name
- (funcall (car fun-list) id)
- (expand-file-name org-attach-id-dir)))
- (setq fun-list (cdr fun-list)))
- (if (file-directory-p attach-dir)
- attach-dir
- attach-dir-preferred))
- attach-dir-preferred)))
+Try id-to-path functions in `org-attach-id-to-path-function-list'
+ignoring nils. If EXISTING is non-nill then return the first path
+found in the filesystem. Otherwise return the first non nil value."
+ (let ((fun-list org-attach-id-to-path-function-list)
+ (base-dir (expand-file-name org-attach-id-dir))
+ preferred first)
+ (while (and fun-list
+ (not preferred))
+ (let* ((name (funcall (car fun-list) id))
+ (candidate (and name (expand-file-name name base-dir))))
+ (setq fun-list (cdr fun-list))
+ (when candidate
+ (if (or (not existing) (file-directory-p candidate))
+ (setq preferred candidate)
+ (unless first
+ (setq first candidate))))))
+ (or preferred first)))
(defun org-attach-check-absolute-path (dir)
"Check if we have enough information to root the attachment directory.
--
2.25.1
