[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] ol.el: add description format parameter to org-link-param
From: |
Ihor Radchenko |
Subject: |
Re: [PATCH v2] ol.el: add description format parameter to org-link-parameters |
Date: |
Thu, 07 Apr 2022 13:13:24 +0800 |
Hugo Heagren <hugo@heagren.com> writes:
> I've added the condition-case back to the check on
> `org-link-make-description', and added a new one to the check for the
> `:default-description' parameter, as Ihor suggested. I've also
> modified the handling of that parameter, to reflect
> `org-link-make-description', and updated the docstring accordingly.
Thanks!
Some more comments:
> + (condition-case nil
> + (let ((def (org-link-get-parameter
> + type
> + :default-description)))
> + (cond
> + ((stringp def) def)
> + ((functionp def)
> + (funcall def link desc))))
> + (error
> + (message "Can't get link description from %S"
> + (symbol-name def))
Firstly, def will be undefined inside the error clause.
Secondly, when :default-description is a lambda expression and that
expression fails, your code will fail with "wrong-type-argument symbolp"
when executing (symbol-name def). Please consider cases when `def' is
not a string and not a function symbol.
I recommend writing tests for org-insert-link in testing/lisp/test-ol.el
and testing expected behaviour for various values of
:default-description.
> Apologies if the subject formatting is not correct, I'm still getting
> the hang of git-send-email.
You don't have to use git-send-email to submit simple patches. An easier
(as for me) alternative is a simple email with attached patch. magit
makes it trivial to create patch files.
Best,
Ihor