Re: [PATCH v9] ol.el: add description format parameter to org-link-parameters
Hugo Heagren writes: > Hugo > From 039c2131462f5454f2804e809e085a055b5bf552 Mon Sep 17 00:00:00 2001 > From: Hugo Heagren > Date: Mon, 28 Mar 2022 23:18:45 +0100 > Subject: [PATCH 1/2] ol.el: add description format parameter to > org-link-parameters > ... > From 708916f8d4f31af1e786e87154b5a4479d0ed1b8 Mon Sep 17 00:00:00 2001 > From: Hugo Heagren > Date: Sat, 16 Jul 2022 19:50:15 +0100 > Subject: [PATCH 2/2] test-ol: tests for insert-description param when > inserting links I have addressed the further comments (mine and Max's) myself after applying your patch. Applied onto main via 2bbb92a72 and e3a05d09b. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e3a05d09b7398b46e8ef724ae7609eeb8a35346e https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=2bbb92a72d4a1be8e56acd41a9c94f5f0023fb28 You are now officially a contributor ;) 🎇 -- Ihor Radchenko, 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
Re: [PATCH v9] ol.el: add description format parameter to org-link-parameters
Adding Org ML back to CC. Hugo Heagren writes: >> The rx-to-string call may still suffer from the described edge case. > > Yes, I think this is a good idea. > >> (and all-prefixes ...) > > I'm not sure what you mean, but I don't think it will work. I mean (and ,all-prefixes (rx-to-string `(: string-start (submatch (or (and ,@all-prefixes))) ":"))) Best, Ihor
Re: [PATCH v9] ol.el: add description format parameter to org-link-parameters
Hugo Heagren writes: > 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)) ":")) > ` > ... > 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? The tests are passing on my side. However, even though you fixed the tests, you did nothing to fix the actual problem revealed by the tests. The rx-to-string call may still suffer from the described edge case. Why not simply shield the rx-to-string call with (and all-prefixes ...)? I'd leave the previous version of the tests as they had a benefit of testing this edge case. Best, Ihor
