Re: [PATCH v2] org-attach.el: ID to path functions may return nil

[Max Nikulin]

On 15/11/2022 09:41, Ihor Radchenko wrote:
> Max Nikulin writes:
> > Please, remove a stray space in the defcustom.
>
> Hmm. Done.
>
> I just have a habit to add space in the first item in a list because it
> helps auto-indentation.

I had an impression that something was wrong with indentation of next 
entries.

> > > 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.

> Handling too short IDs is a different issue indeed, but why not to fix
> it as well?

I still have a different opinion, but it should not prevent you from 
committing your variant. The issue is not critical.

The problem is not too short ID. Actually they are some ID having 
unknown format, so functions designed for particular ID generation 
methods may cause a mess in attachment subdirs.

> > If strict variants of functions were used above then non-standard IDs
> > would be isolated in the directory returned by the next entry
>
> Good point.
> What about using the value you provided in the NEWS as an actual default?

It may be done by a next patch unless other issues will attract more 
attention.

> > `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.
>
> Agree. Fixed.

> +(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)))
> +        ;; Fallback to handle previous defaults.
> +        org-attach-id-uuid-folder-format
> +        org-attach-id-ts-folder-format
> +        ;; Any other ID goes into "important" folder.
> +        (lambda (id) (format "important/%s/%s" (substring id 0 1) id))))

Sorry, but "important" entry should be before 
`org-attach-id-uuid-folder-format'. My idea is the following:
- If the ID was generated by 'uuid or 'org `org-id-method' then prefer 
XX/... subfolder. It is important for new attachments. Even if current 
`org-id-method' is 'ts then this entry is ignored and layout is 
determined by the next entry.
- If the ID is timestamp-based then prefer "YYYYMM/DDTIME" subfolder.
- Put new non-standard IDs to important/X/... subfolder whenever 
particular ID is a short or a long one.
- Handle the case of `org-id-method' changed in the past from timestamp 
to UUID or vice versa. Try to find attachment trying to split 2 
characters from timestamp or 6 characters from UUID. So add bare 
`org-attach-id-uuid-folder-format' and `org-attach-id-ts-folder-format'. 
These 2 entries may be skipped for new users or for users who never 
experienced change of `org-id-method'.