all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: Max Nikulin <manikulin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] org-attach.el: ID to path functions may return nil
Date: Tue, 08 Nov 2022 05:08:04 +0000	[thread overview]
Message-ID: <87y1sm11aj.fsf@localhost> (raw)
In-Reply-To: <tkbdth$qh9$1@ciao.gmane.io>

Max Nikulin <manikulin@gmail.com> writes:

>> AFAIU, your version of `org-attach-dir-from-id' may sometimes return
>> nil, which is neither documented
>
> "otherwise independently of TRY-ALL
> value return the first non nil value."
>
> indirectly assumes nil. It may be made more prominent.

I feel like this makes the docstring confusing.

Note that `org-attach-id-to-path-function-list':

    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.

The docstring implies that only the first function will be used to
create new attachment folders. With your patch, it will no longer be the
case and may surprise users.

You at least need to update the docstring.

> 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, otherwise independently of TRY-ALL
> value return the first non nil value.

This is a very long sentence and I cannot clearly parse which sub-clause
is referenced by "otherwise".

>> nor expected by the callers.
>
> I see 2 direct callers and I am not aware of any problem with them.
>
>> In particular, `org-attach-dir-get-create' will throw unhelpful "No
>> attachment directory is associated with the current node" error.
>
> * H short
> :PROPERTIES:
> :ID:       ec
> :END:
>
> 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.

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

-- 
Ihor Radchenko // yantar92,
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>


  reply	other threads:[~2022-11-08  5:08 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-20 19:12 [BUG] org-attach-id-ts-folder-format fails on customized IDs [9.6 (9.6-??-2e9999783)] Janek F
2022-07-23  5:22 ` [PATCH] " Ihor Radchenko
2022-08-02 22:26   ` Janek F
2022-08-03 16:03   ` Max Nikulin
2022-08-03 22:25     ` Ihor Radchenko
2022-08-10 11:43       ` [PATCH v2] " Ihor Radchenko
2022-08-10 12:17         ` Max Nikulin
2022-08-10 13:23           ` [PATCH v3] " Ihor Radchenko
2022-08-10 15:18             ` Max Nikulin
2022-08-11  4:19               ` Ihor Radchenko
2022-08-11 14:43                 ` Max Nikulin
2022-08-13  5:29                   ` Ihor Radchenko
2022-08-13 16:25                     ` Max Nikulin
2022-08-14  4:00                       ` Ihor Radchenko
2022-10-02  9:14                         ` [PATCH v4] " Ihor Radchenko
2022-10-04 15:27                           ` Max Nikulin
2022-10-05  5:44                             ` Ihor Radchenko
2022-11-06  7:43                               ` Ihor Radchenko
2022-11-07 17:05                               ` [PATCH] org-attach.el: ID to path functions may return nil Max Nikulin
2022-11-08  5:08                                 ` Ihor Radchenko [this message]
2022-11-09 16:53                                   ` [PATCH v2] " Max Nikulin
2022-11-10  7:19                                     ` Ihor Radchenko
2022-11-13 16:26                                       ` Max Nikulin
2022-11-14  3:29                                         ` Ihor Radchenko
2022-11-14 16:59                                           ` Max Nikulin
2022-11-15  2:41                                             ` Ihor Radchenko
2022-11-15 16:41                                               ` Max Nikulin
2022-11-16  1:54                                                 ` Ihor Radchenko
2022-08-12 15:35 ` [BUG] org-attach-id-ts-folder-format fails on customized IDs [9.6 (9.6-??-2e9999783)] Max Nikulin
2022-08-12 16:08   ` Janek F
2022-08-13  5:17     ` Ihor Radchenko
2022-08-13 15:59     ` Max Nikulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y1sm11aj.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=emacs-orgmode@gnu.org \
    --cc=manikulin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.