emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9] ol.el: add description format parameter to org-link-param


From: Hugo Heagren
Subject: Re: [PATCH v9] ol.el: add description format parameter to org-link-parameters
Date: Fri, 29 Jul 2022 20:05:13 +0100

> Hugo, almost certainly you are tired by so many iterations, but I
> still can not approve your patch.

No worries! I has to be right before it can be merged. I wouldn't want
to submit faulty code. And besides, debugging today makes my code
tomorrow better right? This was an interesting case.


> signal(error ("rx form ‘or’ requires at least 1 args"))
> apply(signal (error ("rx form ‘or’ requires at least 1 args")))

The test fails because of an error in `rx-to-string', which is called
by `org-insert-link'. It was failing because I have the following
code:

,----
| (rx-to-string `(: string-start (submatch (or ,@all-prefixes)) ":"))
`----

> It is again Emacs-26

In the final test case, `all-prefixes' is nil at this point, and
before Emacs 27.1, the rx form `or' /required/ arguments (it no longer
does).

So we need to fix the last case so that `or' has some arguments
(`all-prefixes' is non-nil).

The point of the last case is to test the behaviour when there is no
relevant parameter for the link type. I /had/ done this by removing
all the link parameters, but this makes `all-prefixes' nil, so we
can't do that.

We could just as easily do it by leaving the parameters as they are,
and using a link 'type' which is definitely not in the list. I have
taken this approach in the new version of the patch. I've used
"fake-link-type", which will surely not be used, even in anyone's
strange personal config. Admittedly it /could/ be used though (it
would be possible to add it if someone wanted), so if you'd rather, I
can develop something which uses a fake link type which is /by
definition/ not in `org-link-parameters', it would just be rather a
lot more work and the test case might subsequently be less clear to
understand.

Hope that helps -- do the tests pass for you now?

Hugo

Attachment: 0001-ol.el-add-description-format-parameter-to-org-link-p.patch
Description: Text Data

Attachment: 0002-test-ol-tests-for-insert-description-param-when-inse.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]