From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id kGKcL9nVcmNzTAEAbAwnHQ (envelope-from ) for ; Tue, 15 Nov 2022 00:57:13 +0100 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id uFd6L9nVcmMKugAAauVa8A (envelope-from ) for ; Tue, 15 Nov 2022 00:57:13 +0100 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 7782087A8 for ; Tue, 15 Nov 2022 00:57:13 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ouiqb-0003K0-8Q; Mon, 14 Nov 2022 18:26:37 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ouihW-0003Da-TV for emacs-orgmode@gnu.org; Mon, 14 Nov 2022 18:17:20 -0500 Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ouQ9e-0001Uo-9U for emacs-orgmode@gnu.org; Sun, 13 Nov 2022 22:29:05 -0500 Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 29773240101 for ; Mon, 14 Nov 2022 04:28:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1668396539; bh=22cYcxDN2cdyPGgFQI/KdkKLz116FpVFefTRw1K7JFQ=; h=From:To:Cc:Subject:Date:From; b=BUFH+KLJltTnpqZjHMnf2Had/T+ZxNCwtCOaJVSxHJyB19w4g8122opXEoP0jc8e6 j/0M0JZj7aN/VdLCw+y+L19jyBo6c34M/4kfJInog0CMO4wIg2RX7/CTn8QvFcxWcq ovbKjgWfjPLAXUEHINY+ulXEXtAmeYsn7pAwk3R8TEv0vcFFlzWrnOwm1JH/wOG0PU wb/E40oNMJgTqhGX2KrtoDI2K9/YMfYiANfifF0rCaOtsxbZDdrabIUuSUqBTWlEVR dM0XYKPY1irGLZYfQvufUaIYw1gYMZddMQRXqKkV6Jr8rVaE/EF+xE2IxT/ULTgcLe ZM3E1XRuOCbow== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4N9ZXj1QV4z9rxG; Mon, 14 Nov 2022 04:28:52 +0100 (CET) From: Ihor Radchenko To: Max Nikulin Cc: emacs-orgmode@gnu.org Subject: Re: [PATCH v2] org-attach.el: ID to path functions may return nil In-Reply-To: References: <87k084v1wa.fsf@localhost> <871qtxhsm6.fsf@localhost> <87a68ce32u.fsf@localhost> <871qtodygs.fsf@localhost> <87v8qz9zui.fsf@localhost> <35cbf452-c3ed-d97f-db96-dcae57463eff@gmail.com> <87wnbc7ltm.fsf@localhost> <87bksn1nl1.fsf@localhost> <87sfk6zixs.fsf@localhost> <874jwiakpn.fsf@localhost> <87y1sm11aj.fsf@localhost> <87a64zjmz1.fsf@localhost> Date: Mon, 14 Nov 2022 03:29:34 +0000 Message-ID: <87sfimfc2p.fsf@localhost> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: pass client-ip=185.67.36.66; envelope-from=yantar92@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1668470233; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=OVo9ZI8dney//tDwMoAo2FdF+flZDPh7iZJIS/OLQ8U=; b=amL6vi/JVUBT8hRTx5T3Iez+v8by9xVq22z/4IEBFfxRxmSe2SmblAp9iiuqGyNbxAkeQb CfLTOlhVukUHrkAshL6RHFbGHWhHPqFc30xEvV4zgcI6n38mPSmqP8LYjixHN3oKVCwn92 7Of8nk5HVDu4WCS9LMozBwq7sm6joOJI39eD/frbUvXqbBjF+oqw++Jy5uM3rHsY0m3i+e pOgE9Uoe9SNyH0vJO1GG4DaDvaiet8I9RnUldvIR/ZX4RYIT5lVI8g8dmg951iGsCUHIrM f2snfMAXU+UQaQzz0zlJR0j3vKbvHgw2UHGXZw5i2SrCE2DtpOm8eo8pNS9Fgg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1668470233; a=rsa-sha256; cv=none; b=nP0NlHDiQrpKsBTmN51vSnxV4zkAQKLo+4iyVVET/L3gNFJ/kVODaQHvmtHUhhrQKNfQ2r O4YqXLDkS3JTbgvejAymdMhcHwemst62eEpobky1rIF16x+VRngsio+iKPRsUAIOeNzr0y tildIoQ/ZNkx6aeATDrnn5vO5hJYzt3EMEnGGzLav3PM/ERwkVz7yL5EiTL660k033/u4+ i70T6foBj1JMkSzxYs/ZhsEXX8NF954C9N1I2mHeGHeyTSU6ENIKIZUzN0R1IxjXpliAyX bg1cf3ocZEafnUwjc83px3Qccj2gB2LKoQVgIsO23uzxQhpoQLjGTi9WyfUD1w== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=BUFH+KLJ; dmarc=pass (policy=none) header.from=posteo.net; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -3.54 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=posteo.net header.s=2017 header.b=BUFH+KLJ; dmarc=pass (policy=none) header.from=posteo.net; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 7782087A8 X-Spam-Score: -3.54 X-Migadu-Scanner: scn1.migadu.com X-TUID: POZe0ETXELP+ --=-=-= Content-Type: text/plain Max Nikulin writes: >> However, please add NEWS entry detailing the change in >> `org-attach-id-to-path-function-list' to the next version of the patch. > > See the attachment. Thanks! I went through the patch and tried to clarify the wording. Especially in the defcustom docstring. I also added the dumb fallback to the default value. I feel that otherwise the description of too confusing. Let me know what you think about the attached new version of the patch with my amendments. --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: inline; filename=v4-0001-org-attach.el-ID-to-path-functions-may-return-nil.patch Content-Transfer-Encoding: quoted-printable >From aee92ee992d9497c91a0d2bef7a1517df5a032a6 Mon Sep 17 00:00:00 2001 Message-Id: From: Max Nikulin Date: Mon, 7 Nov 2022 23:48:02 +0700 Subject: [PATCH v4] 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): Signal an error suggesting customization of `org-attach-id-to-path-function-list' if all ID-to-path functions return nil. (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. * etc/ORG-NEWS: Advertise the change. 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 Link: https://list.orgmode.org/KC8PcypJapBpJQtJxM0kX5N7Z0THL2Lq6EQjBMzpw1-v= gQf72egZ2JOIlTbPYiqAVD4MdSBhrhBZr2Ykf5DN1mocm1ANvvuKKZShlkgzKYM=3D@pm.me --- etc/ORG-NEWS | 41 ++++++++++++++++++ lisp/org-attach.el | 104 +++++++++++++++++++++++++++++---------------- 2 files changed, 108 insertions(+), 37 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 04b5be64a..851658082 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -604,6 +604,47 @@ With the new customization option ~org-texinfo-with-la= tex~ set to (its default value) ~'detect~, if the system runs Texinfo 6.8 (3 July 2021) or newer, Org will export all LaTeX fragments and environments using Texinfo ~@math~ and ~@displaymath~ commands respectively. +*** More flexible ~org-attach-id-to-path-function-list~ + +List entries may return nil if they are unable to handle passed ID. +So responsibility is passed to the next item in the list. Default +entries ~org-attach-id-uuid-folder-format~ and +~org-attach-id-ts-folder-format~ now return nil in the case of too +short ID. Earlier it caused an obscure error. + +After the change, error text suggests to adjust +~org-attach-id-to-path-function-list~ value. The +~org-attach-dir-from-id~ function is adapted to ignore nil values and +to take first non-nil value instead of the value returned by first +~org-attach-id-to-path-function-list~ item. + +New policy allows to mix different ID styles while keeping subfolder layout +suited best for each one. For example, one can use the following +snippet to allow multiple different ID formats in Org files. + +#+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\\}" i= d) + (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)))) +#+end_src + +If you have 1 or 2 characters long IDs and you do not care what +subdirs are created for longer IDs (that are neither UUIDs nor +timestamp-based) then you may just append the +~org-attach-id-fallback-folder-format~ to +~org-attach-id-to-path-function-list~. It directs attachments to +=3D__=3D=E2=80=8B/ID directory. =20 * Version 9.5 =20 diff --git a/lisp/org-attach.el b/lisp/org-attach.el index ef183474e..e956cac18 100644 --- a/lisp/org-attach.el +++ b/lisp/org-attach.el @@ -166,28 +166,56 @@ (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))) + (and (< 2 (length id)) + (format "%s/%s" + (substring id 0 2) + (substring id 2)))) =20 (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))) - -(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." + (and (< 6 (length id)) + (format "%s/%s" + (substring id 0 6) + (substring id 6)))) + +(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)) + +(defcustom org-attach-id-to-path-function-list '( org-attach-id-uuid-folde= r-format + org-attach-id-ts-folder-format + org-attach-id-fallback-folder-form= at) + "List of functions used to derive attachment path from an ID string. +The functions are called with a single ID argument until the return +value is an existing folder. If no folder has been created yet for +the given ID, then the first non-nil value defines the attachment +dir to be created. + +Usually, the ID format passed to the functions is defined by +`org-id-method'. It is advised that the first function in the list do +not generate all the attachment dirs inside the same parent dir. Some +file systems may have performance issues in such scenario. + +Care should be taken when customizing this variable. Previously +created attachment folders might not be correctly mapped upon removing +functions from the list. Then, Org will not be able to detect the +existing attachments." :group 'org-attach - :package-version '(Org . "9.3") + :package-version '(Org . "9.6") :type '(repeat (function :tag "Function with ID as input"))) =20 (defvar org-attach-after-change-hook nil @@ -360,7 +388,7 @@ (defun org-attach-dir (&optional create-if-not-exists-p= no-fs-check) (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 +409,11 @@ (defun org-attach-dir-get-create () (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 +424,25 @@ (defun org-attach-dir-get-create () (make-directory attach-dir t)) attach-dir)) =20 -(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-nil, 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))) =20 (defun org-attach-check-absolute-path (dir) "Check if we have enough information to root the attachment directory. --=20 2.35.1 --=-=-= Content-Type: text/plain -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at . Support Org development at , or support my work at --=-=-=--