Max Nikulin <maniku...@gmail.com> writes: > On 08/11/2022 12:08, Ihor Radchenko wrote: >> >> I feel like this makes the docstring confusing. >> >> Note that `org-attach-id-to-path-function-list': > > I have tried to update the docstrings.
Thanks! >>> 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. > > I have moved `error' call despite I have not figured out how to trigger > the error for other options. The main reason is code readability. Also, one can trigger the other error for non-standard values of `org-attach-preferred-new-method' . >>> +(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. > > I am still unsure that in this case effect is more important than > purpose. The function is too specific. That's a function and should follow standards. If we do not follow standards, it will become harder to maintain Org. If we want to be really specific, we may allow a special symbol in the list indicating the fallback. I'd prefer this approach a bit better. No strong opinion though. > P.S. At first I believed that you have some objections concerning > changed role of the first function in the list, not just how it is > documented. I had. Most importantly, because we are changing the existing meaning of `org-attach-id-to-path-function-list'. However, the only scenario where it actually matters is the bug report at hand. So, there will be no regression. However, please add NEWS entry detailing the change in `org-attach-id-to-path-function-list' to the next version of the patch. I have no other comments apart from various grammar issues and typos. But that's easy to fix before merging. -- 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>
