Re: [PATCH v2] Re: [BUG] org-attach-id-ts-folder-format fails on customized IDs [9.6 (9.6-??-2e9999783)]

[Max Nikulin]

On 10/08/2022 18:43, Ihor Radchenko wrote:

Ihor Radchenko writes:

I have updated the patch to use "__/id" when id is too short.
Any objections?

+When ID is too short (less than 3 chars), use its md5 hash to create


Misleading docs, you do not use md5.


+the path."
+  (if (< (length id) 3)
+  (format "--/%s" id)


Please, do not use path components starting with dash, it is terrible 
for CLI tools. By the way, you promised underscores, not dashes.



+(format "%s/%s"
+   (substring id 0 2)
+   (substring id 2


Ihor, I have not look into the code around, so my suggestions may have 
no sense.


Is it possible to pass empty string as ID to these functions? Should it 
be explicitly checked?


What if ID contains "/" that can not be used in file name? Windows has 
more forbidden characters.


Do you expect any problem if here (and for timestamp-based ids) 
directory component is just padded with some character (unsure what can 
better than underscore) to required length?


"x" -> "_x/x" or "__x/x"
"xy" -> "xy/xy" or "_xy/xy"
etc.

From my point of view it might be a bit better than "__" and "unknown".

[PATCH v2] Re: [BUG] org-attach-id-ts-folder-format fails on customized IDs [9.6 (9.6-??-2e9999783)]

[Ihor Radchenko]

Ihor Radchenko  writes:

>> Ihor, I am afraid of collisions due to short input to md5. Long hash 
>> value gives false impression of high entropy but actually there are not 
>> so many variants for 1 or 2 characters strings. Either there is no point 
>> in making long name from short id or random string of appropriate length 
>> should be used instead.
>
> Random is not an option. If we use random string, how will we get the
> right attachment path on a second call? This function must return the
> same value given the same input.
>
> Having said that, I do agree (taking into account the other comment)
> that md5 may not be a very good idea. Maybe simply something like
> "__/id". Ideally, we should be able to recover the original id from the
> path to attachment.

I have updated the patch to use "__/id" when id is too short.
Any objections?

>From 7ca5fe66c53a1399943a2e2d4a1675ca8cd14194 Mon Sep 17 00:00:00 2001
Message-Id: <7ca5fe66c53a1399943a2e2d4a1675ca8cd14194.1660131747.git.yanta...@gmail.com>
From: Ihor Radchenko 
Date: Sat, 23 Jul 2022 13:13:24 +0800
Subject: [PATCH v2] org-attach-dir-from-id: Do not rely on ID being over 6
 chars long

* lisp/org-attach.el (org-attach-id-uuid-folder-format): Fall back to
"--/ID" when the ID contains 2 chars or less and cannot be split into
the "xy/z" path.
(org-attach-id-ts-folder-format): Fall back to "unknown/ID" path
format when the ID contains less than 7 chars and cannot be split into
the "MM/rest" path.

Fixes https://orgmode.org/list/KC8PcypJapBpJQtJxM0kX5N7Z0THL2Lq6EQjBMzpw1-vgQf72egZ2JOIlTbPYiqAVD4MdSBhrhBZr2Ykf5DN1mocm1ANvvuKKZShlkgzKYM=@pm.me
---
 lisp/org-attach.el | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index fe49af6f3..bb7c787f7 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -159,19 +159,28 @@ (defcustom org-attach-archive-delete nil
 (defun org-attach-id-uuid-folder-format (id)
   "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)))
+attachments.  Useful if ID is generated with UUID.
+
+When ID is too short (less than 3 chars), use its md5 hash to create
+the path."
+  (if (< (length id) 3)
+  (format "--/%s" 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)))
+year-month, the rest.
+
+When ID is too short (less than 7 chars), return \"unknown/ID\"."
+  (if (< (length id) 7)
+  (format "unknown/%s" id)
+(format "%s/%s"
+	(substring id 0 6)
+	(substring id 6
 
 (defcustom org-attach-id-to-path-function-list '(org-attach-id-uuid-folder-format
 		 org-attach-id-ts-folder-format)
-- 
2.35.1



-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92