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

2022-08-03 Thread Ihor Radchenko
Max Nikulin  writes:

> On 23/07/2022 12:22, Ihor Radchenko wrote:
>> Tentative patch is attached.
>
>> +  (if (< (length id) 3)
>> +  (org-attach-id-uuid-folder-format (md5 id))
>
> 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.

Best,
Ihor



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

2022-08-03 Thread Max Nikulin

On 23/07/2022 12:22, Ihor Radchenko wrote:

Tentative patch is attached.



+  (if (< (length id) 3)
+  (org-attach-id-uuid-folder-format (md5 id))


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.





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

2022-08-02 Thread Janek F
Just now had a closer look at your patch,
and I don't like the hashing.
My whole idea customizing ids is to make the file structure useful by itself, 
too.
This is how I patched it for me:

  (defun xf/org-attach-id-folder-format (id)
"Translate any ID into a folder-path."
(format "%s/%s"
(substring id 0 2)
(if (> (seq-length id) 2) (substring id 2) id))
)
  (setq org-attach-id-to-path-function-list '(xf/org-attach-id-folder-format))


--- Original Message ---
Ihor Radchenko  schrieb am Samstag, 23. Juli 2022 um 07:21:


> Janek F xer...@pm.me writes:
>
> > When setting org-id-method to 'ts or 'org,
> > org-attach seems to use org-attach-id-ts-folder-format
> > to create its hierarchy.
> >
> > However I tend to customize IDs for important files by hand,
> > causing any attempt to use org-attach on that file to fail
> > if the ID is shorter than six characters:
> >
> > org-attach-id-ts-folder-format: Args out of range: "ftt", 0, 6
> >
> > This method should be adjusted to handle non-ts-ids just as well,
> > as org-id-method does not dictate the format of existing ids.
>
>
> Thanks for reporting!
> Tentative patch is attached.
>
> I have added some fallbacks for the default attach folder formatters, so
> that they can work when the ID does not conform to specific format.
>
> This change is somewhat opinionated, so feel free to suggest alternative
> solutions/fallback options.
>
> Best,
> Ihor



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

2022-07-22 Thread Ihor Radchenko
Janek F  writes:

> When setting org-id-method to 'ts or 'org,
> org-attach seems to use org-attach-id-ts-folder-format
> to create its hierarchy.
>
> However I tend to customize IDs for important files by hand,
> causing any attempt to use org-attach on that file to fail
> if the ID is shorter than six characters:
>
>     org-attach-id-ts-folder-format: Args out of range: "ftt", 0, 6
>
> This method should be adjusted to handle non-ts-ids just as well,
> as org-id-method does not dictate the format of existing ids.

Thanks for reporting!
Tentative patch is attached.

I have added some fallbacks for the default attach folder formatters, so
that they can work when the ID does not conform to specific format.

This change is somewhat opinionated, so feel free to suggest alternative
solutions/fallback options.

Best,
Ihor

>From e004752ba39f0d328645a1f6053ad3ce3b06ac28 Mon Sep 17 00:00:00 2001
Message-Id: 
From: Ihor Radchenko 
Date: Sat, 23 Jul 2022 13:13:24 +0800
Subject: [PATCH] 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
using ID md5 hash 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 36c21b702..7c72fd7ee 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)
+  (org-attach-id-uuid-folder-format (md5 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