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

[Ihor Radchenko]

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

[Ihor Radchenko]

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

[Ihor Radchenko]

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

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

[Max Nikulin]

On 25/07/2022 18:55, Hugo Heagren wrote:

In any case, we cannot, unfortunately, use map-do here and should
probably fall back to the previous version with cl-loop.


You're right, this is strange. Oh well, I've moved back to the version
of the macro which used `cl-loop' in the current patch. Nothing else
is changed. All the tests pass for me.


Hugo, almost certainly you are tired by so many iterations, but I still 
can not approve your patch. It is again Emacs-26 and likely the last 
`should' form with (org-link-parameters nil). I hope, you have not lost 
motivation yet and you will fix this test failure.


Test test-ol/insert-link-insert-description backtrace:
  signal(error ("rx form ‘or’ requires at least 1 args"))
  apply(signal (error ("rx form ‘or’ requires at least 1 args")))
  (setq value-7590 (apply fn-7588 args-7589))
  (unwind-protect (setq value-7590 (apply fn-7588 args-7589)) (setq fo
  (if (unwind-protect (setq value-7590 (apply fn-7588 args-7589)) (set
  (let (form-description-7592) (if (unwind-protect (setq value-7590 (a
  (let ((value-7590 (quote ert-form-evaluation-aborted-7591))) (let (f
  (let* ((fn-7588 (function null)) (args-7589 (condition-case err (let
  (closure (t) nil (let* ((fn-7563 (function string=)) (args-7564 (con
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name test-ol/insert-link-insert-descriptio
  ert-run-or-rerun-test(#s(ert--stats :selector "test-ol/" :tests [#s(
  ert-run-tests("test-ol/" #f(compiled-function (event-type  even
  ert-run-tests-batch("test-ol/")
  ert-run-tests-batch-and-exit("test-ol/")
  (let ((org-id-track-globally t) (org-test-selector (if org-test-sele
  org-test-run-batch-tests("test-ol/")
  eval((org-test-run-batch-tests org-test-select-re))
  command-line-1(("--eval" "(setq vc-handled-backends nil org-startup-
  command-line()
  normal-top-level()
Test test-ol/insert-link-insert-description condition:
(error "rx form ‘or’ requires at least 1 args")
   FAILED   5/13  test-ol/insert-link-insert-description

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

[Bastien Guerry]

Hi Ihor and Hugo,

Ihor Radchenko  writes:

> Bastien, can you please confirm that Hugo is listed in the FSF
> records

Yes I do!

> and then add him to the contributors.org?

Done.

-- 
 Bastien

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

[Ihor Radchenko]

Hugo Heagren  writes:

> On 18/07/2022 17:55, Max Nikulin wrote:
>
>> I am sorry if I am wrong, but I do not see you among known Org, 
>> contributors. You patch is likely greater than it is allowed for
>> TINYCHANGE, so before you patch can be committed, copyright assignment
>> should be signed, see
>>  for details.
>
> I've already assigned copyright to the FSF for work on Emacs (see
> attached certificate). Do I need to do anything else for org-mode in
> particular?

Bastien, can you please confirm that Hugo is listed in the FSF records
and then add him to the contributors.org?

Best,
Ihor

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

[Ihor Radchenko]

Max Nikulin  writes:

>>> It is `map-do' that causes the problem. Maybe earlier implementation in
>>> Emacs-26 has some peculiarities. It is the version available from the
>>> system repository for Ubuntu-20.04 LTS focal.
>> 
>> Tests do not fail on my side using Emacs 26.3
>
> Ihor, what is the origin of your Emacs-26.3? The patch uses `map-do' 
> with plist argument. I have tried:
>
> test-map-do.el
>  >8 
> (require 'map)
> (map-do (lambda (k v) (message "alist map-do: %S: %S" k v)) '((1 . 'a)))
> (setq pl nil)
> (setq pl (plist-put pl :a 'a))
> (message "plist: %S" pl)
> (map-do (lambda (k v) (message "plist map-do %S: %S" k v)) pl)
>  8< 

It also fails on my side.

Yet,
make test EMACS="emacs-26" does _not_ fail.

I tried harder then and found that
make test EMACS="emacs-26" BTEST_RE="test-ol/insert-link-insert-description"
_does_ fail.

> So I am puzzled how tests using a macro with map-do and plist may pass 
> under Emacs-26.

I am also puzzled now. Running all the tests works, while running a
single test does not.

In any case, we cannot, unfortunately, use map-do here and should
probably fall back to the previous version with cl-loop.

Best,
Ihor

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

[Max Nikulin]

On 23/07/2022 20:06, Ihor Radchenko wrote:

Max Nikulin writes:


If it still fails, could you try isolating whether it's a specific
`should' clause which is the problem, if it fails with any of them?


It is `map-do' that causes the problem. Maybe earlier implementation in
Emacs-26 has some peculiarities. It is the version available from the
system repository for Ubuntu-20.04 LTS focal.


Tests do not fail on my side using Emacs 26.3


Ihor, what is the origin of your Emacs-26.3? The patch uses `map-do' 
with plist argument. I have tried:


test-map-do.el
 >8 
(require 'map)
(map-do (lambda (k v) (message "alist map-do: %S: %S" k v)) '((1 . 'a)))
(setq pl nil)
(setq pl (plist-put pl :a 'a))
(message "plist: %S" pl)
(map-do (lambda (k v) (message "plist map-do %S: %S" k v)) pl)
 8< 

Emacs-26.3: failure
emacs -Q --batch -l test-map-do.el
alist map-do: 1: (quote a)
plist: (:a a)
Wrong type argument: listp, :a

Emacs-27: works
emacs -Q --batch -l test-map-do.el
alist map-do: 1: 'a
plist: (:a a)
plist map-do :a: a

It is consistent with
f68f2eb472 2018-12-20 08:40:43 -0500 Stefan Monnier: * 
lisp/emacs-lisp/map.el: Add support for plists


I see no equivalent change in emacs-26 branch and emacs-26.3 tag.

So I am puzzled how tests using a macro with map-do and plist may pass 
under Emacs-26.

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

[Max Nikulin]

On 23/07/2022 20:06, Ihor Radchenko wrote:

Max Nikulin writes:


If it still fails, could you try isolating whether it's a specific
`should' clause which is the problem, if it fails with any of them?


It is `map-do' that causes the problem. Maybe earlier implementation in
Emacs-26 has some peculiarities. It is the version available from the
system repository for Ubuntu-20.04 LTS focal.


Tests do not fail on my side using Emacs 26.3


It is strange. I tried the latest 2 patches applied on the top of

127e7fee4 2022-07-23 22:16:54 +0800 Ihor Radchenko: ox-latex: Keep 
obsolete variable values removed in 97cfb959d


and I got the same failure as a week ago. Maybe I will try to debug 
`map-do' tomorrow. Previous time my impression was that map-do was 
completely broken. I never used it before, but I assumed that it is used 
in the patch in a proper way. I do not expect that the cause is minimal 
testing environment in a LXC container (e.g. missing XDG directories).

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

[Ihor Radchenko]

Max Nikulin  writes:

>> If it still fails, could you try isolating whether it's a specific
>> `should' clause which is the problem, if it fails with any of them?
>
> It is `map-do' that causes the problem. Maybe earlier implementation in 
> Emacs-26 has some peculiarities. It is the version available from the 
> system repository for Ubuntu-20.04 LTS focal.

Tests do not fail on my side using Emacs 26.3

Best,
Ihor

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

[Max Nikulin]

On 23/07/2022 14:48, Hugo Heagren wrote:


I've already assigned copyright to the FSF for work on Emacs (see
attached certificate). Do I need to do anything else for org-mode in
particular?


Great, I hope, it is enough, but my answer is not authoritative.


Emacs-26.3:
make test-dirty BTEST_RE=test-ol/insert-link-insert-description
[failing test]


If it still fails, could you try isolating whether it's a specific
`should' clause which is the problem, if it fails with any of them?


It is `map-do' that causes the problem. Maybe earlier implementation in 
Emacs-26 has some peculiarities. It is the version available from the 
system repository for Ubuntu-20.04 LTS focal.

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

[Max Nikulin]

Hugo,

I am sorry if I am wrong, but I do not see you among known Org 
contributors. You patch is likely greater than it is allowed for 
TINYCHANGE, so before you patch can be committed, copyright assignment 
should be signed, see 
https://orgmode.org/worg/org-contribute.html#copyright for details.


On 18/07/2022 03:59, Hugo Heagren wrote:

 From fbe030ad3a2aafd09d491aefb9c56242b7ec669b Mon Sep 17 00:00:00 2001
From: Hugo Heagren
Date: Sat, 16 Jul 2022 19:50:15 +0100
Subject: [PATCH] test-ol: tests for insert-description param when inserting
  links


Emacs-26.3:
make test-dirty BTEST_RE=test-ol/insert-link-insert-description

selected tests: test-ol/insert-link-insert-description
Running 1 tests (2022-07-18 12:21:46+0200)
Test test-ol/insert-link-insert-description backtrace:
  signal(wrong-type-argument (listp :insert-description))
  apply(signal (wrong-type-argument (listp :insert-description)))
  (setq value-7565 (apply fn-7563 args-7564))
  (unwind-protect (setq value-7565 (apply fn-7563 args-7564)) (setq fo
  (if (unwind-protect (setq value-7565 (apply fn-7563 args-7564)) (set
  (let (form-description-7567) (if (unwind-protect (setq value-7565 (a
  (let ((value-7565 (quote ert-form-evaluation-aborted-7566))) (let (f
  (let* ((fn-7563 (function signal)) (args-7564 (condition-case err (l
  (closure (t) nil (let* ((fn-7563 (function signal)) (args-7564 (cond
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name test-ol/insert-link-insert-descriptio
  ert-run-or-rerun-test(#s(ert--stats :selector "test-ol/insert-link-i
  ert-run-tests("test-ol/insert-link-insert-description" #f(compiled-f
  ert-run-tests-batch("test-ol/insert-link-insert-description")
  ert-run-tests-batch-and-exit("test-ol/insert-link-insert-description
  (let ((org-id-track-globally t) (org-test-selector (if org-test-sele
  org-test-run-batch-tests("test-ol/insert-link-insert-description")
  eval((org-test-run-batch-tests org-test-select-re))
  command-line-1(("--eval" "(setq vc-handled-backends nil org-startup-
  command-line()
  normal-top-level()
Test test-ol/insert-link-insert-description condition:
(wrong-type-argument listp :insert-description)
   FAILED  1/1  test-ol/insert-link-insert-description


+(ert-deftest test-ol/insert-link-insert-description ()
+  "Test `:insert-description' parameter handling."
+  ;; String case.


The cases might be improved by using different values, so when 
particular `should' form fail it is easier to find it in the code



+  (should
+   (string=
+"foobar"
+(test-ol-with-link-parameters-as
+"id" (:insert-description "foobar")


E.g. "foobar-string"


+  (test-ol-insert-link-get-desc "id:foo-bar"
+  ;; Lambda case.
+  (should
+   (string=
+"foobar"
+(test-ol-with-link-parameters-as
+"id" (:insert-description (lambda (_link-test _desc) "foobar"))
+  (test-ol-insert-link-get-desc "id:foo-bar"


"foobar-lambda"

Further "foobar-desc-arg", etc.


+`:insert-description'
+
+  String or function used as a default when prompting users for a
+  link's description.  A string is used as-is, a function is
+  called with two arguments: the full link text, and the


"link text" might be a bit ambiguous here. I would consider "link 
location", "string containing link type and target", or something else.



+  description generated by `org-insert-link'.  It should return
+  the description to use (this reflects the behaviour of
+  `org-link-make-description-function').  If it returns nil, no
+  default description is used, but no error is thrown (from the
+  user's perspective, this is equivalent to a default description
+  of \"\").

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

[Ihor Radchenko]

Max Nikulin  writes:

> Ihor, Hugo raised a question how to interpret nil returned by 
> :insert-description functions. Do you have any opinion? I do not like 
> the idea to consider it as an error since string is expected. I would 
> prefer to call `org-link-make-description-function' as a fallback.
>
> On 17/07/2022 04:20, Hugo Heagren wrote:
>>> Can you also update the documentation for
>>> org-link-make-description-function?
>> 
>> I'm not sure what sort of documentation you have in mind? What should
>> I add?
>
> I have no idea what Ihor means. From my point of view, mention of 
> :insert-description and `org-link-parameters' may give a hint to users.

Let me clarify.

The new :insert-description parameter is a more fine-grained equivalent
to already existing org-link-make-description-function. The latter can,
de facto, return string or nil - it is passed to `read-string' as
INITIAL-INPUT argument.

So, I suggested interpreting nil return value in :insert-description
just as it is already interpreted in org-link-make-description-function.
Further, I asked to document that nil value is allowed - both in
:insert-description docstring section and in
org-link-make-description-function docstring.

Best,
Ihor

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

[Ihor Radchenko]

Hugo Heagren  writes:

>> Can you also update the documentation for
>> org-link-make-description-function?
>
> I'm not sure what sort of documentation you have in mind? What should
> I add?

I referred to my earlier statement:

> tl;dr The question is: what is the Good Behaviour when
> :default-description is set to something, which is meant to return a
> string and returns 'nil instead? Should it be treated like an empty
> string, or as an error?

Currently, the internal implementation will treat nil return value as if
there was no :default-description and org-link-make-description-function
were set to nil. We may probably document this. It sounds like a useful
behaviour.

If the :default-description function returns non-string and not nil, the
behaviour is simply undefined. It was also the case for
org-link-make-description-function. Though we might add a cl-assert
somewhere near the end of org-insert-link to deliberately throw an
error.

The return value of org-link-make-description-function can also be nil
in addition to string.  I suggest documenting this, just as you did for
:insert-description parameter.

Best,
Ihor

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

[Max Nikulin]

Hi,

Ihor, Hugo raised a question how to interpret nil returned by 
:insert-description functions. Do you have any opinion? I do not like 
the idea to consider it as an error since string is expected. I would 
prefer to call `org-link-make-description-function' as a fallback.


On 17/07/2022 04:20, Hugo Heagren wrote:

Can you also update the documentation for
org-link-make-description-function?


I'm not sure what sort of documentation you have in mind? What should
I add?


I have no idea what Ihor means. From my point of view, mention of 
:insert-description and `org-link-parameters' may give a hint to users.



Have I missed something or the whole macro may be simplified to just
copy `org-link-parameters'?

`(let ((org-link-parameters (copy-tree org-link-parameters)))
   (org-link-set-parameters ,type ,@parameters)
   ,@body))


I had the same thought. I doesn't work though. `copy-tree' and
`copy-sequence' only make shallow copies of each element in a
sequence. `org-link-set-parameters' then uses side effects to alter
the `org-link-parameters', so these changes are propagated to the
copy. This means the values in the copy and the real
`org-link-parameters' are always the same, and so we can't use the
copy to store the original values (which is what we need it to do).


I have tried it again (Emacs-26.3). I agree that `copy-sequence' and 
shallow copy is not enough, but `copy-tree' works for me. It rather a 
curiosity than demanding related changes.


(defmacro nm-with-org-link-parameters (type parameters  body)
  (declare (indent 2))
  `(let ((org-link-parameters (copy-tree org-link-parameters)))
 (org-link-set-parameters ,type ,@parameters)
 ,@body))

(nm-with-org-link-parameters "help" (:insert-description (lambda (_link 
_descr) "h1"))

  (assoc "help" org-link-parameters))
=> ("help" :follow org-link--open-help :store org-link--store-help 
:insert-description (lambda (_link _descr) "h1"))


(assoc "help" org-link-parameters)
=> ("help" :follow org-link--open-help :store org-link--store-help)


I have never used `declare' before. I looked it up in the Elisp manual
and read the docstring, but I couldn't understand how it specifies
which argument is considered the body. I'm also not aware of what this
does/why this is useful? (not a criticism, I just haven't learned this
yet).


See info "(elisp) Indenting Macros". Without `declare' automatic 
indentation is the following:


(nm-with-org-link-parameters "help" (:insert-description (lambda (_link 
_descr) "h1"))

 (assoc "help" org-link-parameters))


and maybe even be a sibling of
 (def (org-link-get-parameter type
to keep related code more local.


This isn't possible, because that's a clause in a `let' call, which is
itself inside a `cond' clause, and the `CONDITION' of that clause uses
`type', so it has to be defined at a higher level.


It was written with assumption that `or' is used instead of `cond' to 
call `org-link-make-description-function' if :insert-description returns 
nil.



I have not tested, so I may be wrong in respect to the following. It
seems, you rises priority of desc value that earlier was a fallback.
The reason why I am in doubts, is that it seems, desc is initialized
from current link description when point is withing an existing link
and in such cases desc likely should be preserved. I can not say
that I like that it is responsibility of all description functions
to return the desc argument if it is supplied.


It's right that `desc' is initialized from current link description
when point is withing an existing link.

Previously, `desc' was only used when
`org-link-make-description-function' was nil. This seems to me a very
odd behaviour: preserve the current link description, but only when
`org-link-make-description-function' is nil. Especially considering
that `org-insert-link' is also used for editing already-existing
links. So in the original version, in some situations,
`org-link-make-description-function' had a higher priority than
`desc', which seems wrong.


In addition to changing behavior, you decision is still a trade-off. 
Consider the case when, editing some link, a user changes link path and 
expects to get new default description. When description function is 
called, to respect existing description, it may be defined as


(lambda (link desc)
 (or (org-string-nw-p desc)
 (format-string "My link %s" link)))

I do not like that in such approach checking of DESC is mandatory, so it 
is OK for me to commit your variant with high priority of DESC and to 
postpone discussion of possible improvements.


A side-note: see the following thread for a feedback form a user who is 
not happy with current prompt for description:
Visuwesh. [BUG] org-insert-link should use DEFAULT in read-string when 
asking for description. Fri, 25 Feb 2022 19:49:23 +0530. 
https://list.orgmode.org/87sfs7jafo@gmail.com



+  (t (condition-case nil
+ 

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

[Ihor Radchenko]

Hugo Heagren  writes:

>> Currently, the internal implementation will treat nil return value as if
>> there was no :default-description and org-link-make-description-function
>> were set to nil. We may probably document this. It sounds like a useful
>> behavior.
>
> I added some relevant documentation to the `:default-description' part
> of the `org-link-parameters' docstring. I also changed the
> previosly-failing test to expect the right value. All the tests pass
> now.

Thanks! Can you also update the documentation for
org-link-make-description-function?

> From cd5268bc05324140a4cf384ce12b99bba1ddab47 Mon Sep 17 00:00:00 2001
> From: Hugo Heagren 
> Date: Mon, 28 Mar 2022 23:18:45 +0100
> Subject: [PATCH] ol.el: add description format parameter to
>  org-link-parameters

Also, can you please review the comments from Max:
https://list.orgmode.org/877d4flu3x@heagren.com/T/#m3b48d0076a25eaf24b1af48df317984dd4e33fb0

> +;; Copy all keys in `parameters' and their original values to
> +;; `orig-parameters'.
> +(cl-loop for param in parameters by 'cddr
> + do (setq orig-parameters
> +  (plist-put orig-parameters param 
> (org-link-get-parameter type param

Thinking about this part a bit more, I have realized that even better
option would be using map-do.

Best,
Ihor

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

[Max Nikulin]

On 08/07/2022 02:57, Hugo Heagren wrote:

Since the last version of this patch, I have:


Thank you, this version should be more reliable.


tl;dr The question is: what is the Good Behaviour when
:default-description is set to something, which is meant to return a
string and returns 'nil instead? Should it be treated like an empty
string, or as an error?
Just an idea: if the :default-description function returns "" then use 
empty description, if it returns nil then try 
`org-link-make-description-function'.


Unsure if it is better but I would consider `or' instead of `cond':

(or description
(let ((make (org-link-get-parameter type :default-description)))
 (and make (condition-case ; ...
)))
(and org-link-make-description-function
 (condition-case ; ...
))
desc)

So it becomes a kind of responsibility chain and nil becomes a valid and 
useful value.


I am in doubts concerning "default-description" as the parameter name. I 
would consider either more specific "insert-description" or shorter 
"description". Concerning the former, sometimes I do not mind to have 
default description for export shared by most of backends without 
necessity to explicitly write :export function handling all backends. 
E.g. for  generate 
https://orgmode.org/manual/Protocols.html target and 'info "(org) 
Protocols"' description that is suitable for LaTeX/PDF, HTML, Markdown. 
If something like this were implemented, default-description would 
become ambiguous if it is related to insert or to export.



+(defmacro test-ol-with-link-parameters-as (type parameters  body)

[...]

+  ;; Copy all keys in `parameters' and their original values to
+  ;; `orig-parameters'. For `parity': nil = odd, non-nill = even
+  `(let (parity orig-parameters)
+ (dolist (p ',parameters)


Have I missed something or the whole macro may be simplified to just 
copy `org-link-parameters'?


  `(let ((org-link-parameters (copy-tree org-link-parameters)))
 (org-link-set-parameters ,type ,@parameters)
 ,@body))

Otherwise `gensym'-generated name should be used instead of hardcoded 
rtn to avoid accidental modification of the variable by the body code:



+ (let ((_ (org-link-set-parameters ,type ,@parameters))
+   ;; Do body
+   (rtn (progn ,@body)))


In addition, `declare' form should be added to `defmacro' to specify 
which argument is considered as its body.



+(setq type
+  (cond


My opinion is that it should be inside
   (let ((initial-input ...
and maybe even be a sibling of
   (def (org-link-get-parameter type
to keep related code more local.


- ((not org-link-make-description-function) desc)
+  (desc)
+  ((org-link-get-parameter type :default-description)
+   (let ((def (org-link-get-parameter type :default-description)))


I have not tested, so I may be wrong in respect to the following. It 
seems, you rises priority of desc value that earlier was a fallback. The 
reason why I am in doubts, is that it seems, desc is initialized from 
current link description when point is withing an existing link and in 
such cases desc likely should be preserved. I can not say that I like 
that it is responsibility of all description functions to return the 
desc argument if it is supplied.



  (t (condition-case nil
 (funcall org-link-make-description-function link desc)


Notice that before you modification `funcall' was guarded by "(not 
org-link-make-description-function)" test.


I like the idea of description specific to link type and it is sour that 
previous attempts to implement the feature was not completed.

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

[Ihor Radchenko]

Hugo Heagren  writes:

> Since the last version of this patch, I have:
> - moved the code which sets `type' in `org-insert-link' to a position
>   where it covers more cases
> - rewritten the macros used in the tests, so that always (and
>   correctly) restore the original state after running, even after
>   errors. Thanks to Max Nikulin for suggesting using `condition-case'

Thanks!

> tl;dr The question is: what is the Good Behaviour when
> :default-description is set to something, which is meant to return a
> string and returns 'nil instead? Should it be treated like an empty
> string, or as an error?

Currently, the internal implementation will treat nil return value as if
there was no :default-description and org-link-make-description-function
were set to nil. We may probably document this. It sounds like a useful
behavior.

If the :default-description function returns non-string and not nil, the
behavior is simply undefined. It was also the case for
org-link-make-description-function. Though we might add a cl-assert
somewhere near the end of org-insert-link to deliberately throw an
error.

> Subject: [PATCH 2/2] test-ol: tests for default-description param when
>  inserting links

> Add tests for various values of `:default-description' in
> `org-link-parameters'.

Could you also add the proper changelog entry for the test-ol.el file?

> +;;; Insert Links
> +
> +(defmacro test-ol-with-link-parameters-as (type parameters  body)
> +  "Pass TYPE/PARAMETERS to `org-link-parameters' and execute BODY.
> +
> +Save the original value of `org-link-parameters', execute
> +`org-link-set-parameters' with the relevant args, execute BODY
> +and restore `org-link-parameters'.
> +
> +TYPE is as in `org-link-set-parameters'.  PARAMETERS is a plist to
> +be passed to `org-link-set-parameters'."
> +  ;; Copy all keys in `parameters' and their original values to
> +  ;; `orig-parameters'. For `parity': nil = odd, non-nill = even

*non-nil

Also, please end each sentence in the comment with ".".

> +  `(let (parity orig-parameters)
> + (dolist (p ',parameters)
> +   (unless parity
> + (setq orig-parameters
> +   (plist-put orig-parameters p (org-link-get-parameter ,type 
> p
> +   (setq parity (not parity)))

This is a bit awkward code. You can instead use cl-loop by 'cddr or
a simple while loop with two (pop parameters) statements inside.

Also, ',parameters will fail if PARAMETERS argument is a variable name.

> + ;; Set `parameters' values
> + (condition-case err
> + (let ((_ (org-link-set-parameters ,type ,@parameters))
> +   ;; Do body
> +   (rtn (progn ,@body)))
> +   ;; Restore original values
> +   (apply 'org-link-set-parameters ,type orig-parameters)
> +   ;; Return whatever the body returned
> +   rtn)
> +   ;; In case of error, restore state anyway AND really error
> +   (error
> +(apply 'org-link-set-parameters ,type orig-parameters)
> +    (signal (car err) (cdr err))

unwind-protect is more suitable here and will lead to more concise code.

> Subject: [PATCH 1/2] ol.el: add description format parameter to
>  org-link-parameters
>
> * ol.el (org-link-parameters): add parameter `:default-description', a
> string or a function.
> * (org-insert-link): if no description is provided (pre-existing or as
> an argument), next option is to use the `:default-description' (if
> non-nil) parameter to generate one.
>
> Default descriptions are predictable within a link type, but because
> link types are quite diverse, are NOT predictable across many types. A
> type-parameter is thus a good place to store information on the
> default description.

Please start sentences after ":" with capital letter. Also, you missed
double space between sentences.

> +`:default-description'
> +
> +  String or function used as a default when prompting users for a
> +  link's description.  A string is used as-is, a function is
> +  called with two arguments: the full link text, and the
> +  description generated by `org-insert-link'.  It should return
> +  the description to use (this reflects the behaviour of
> +  `org-link-make-description-function').
> +
>  `:display'

I think that we should also document the new parameter in ORG-NEWS.
  
Best,
Ihor

[PATCH v4] ol.el: add description format parameter to org-link-parameters

[Hugo Heagren]

rt-link-get-desc "id:foo-bar"
+  ;; Lambda case
+  (should
+   (string=
+"foobar"
+(test-ol-with-link-parameters-as
+ "id" (:default-description (lambda (_link-test _desc) "foobar"))
+ (test-ol-insert-link-get-desc "id:foo-bar"
+  ;; Function symbol case
+  (should
+   (string=
+"foobar"
+(test-ol-with-link-parameters-as
+ "id" (:default-description #'test-ol/return-foobar)
+ (test-ol-insert-link-get-desc "id:foo-bar"
+  ;; `:default-description' parameter is defined, but doesn't return a
+  ;; string
+  (should-error
+   (test-ol-with-link-parameters-as
+"id" (:default-description #'ignore)
+(test-ol-insert-link-get-desc "id:foo-bar")))
+  ;; Description argument should override `:default-description'
+  (should
+   (string=
+"notfoobar"
+(test-ol-with-link-parameters-as
+ "id" (:default-description "foobar")
+ (test-ol-insert-link-get-desc "id:foo-bar" "notfoobar")))))
+
 (provide 'test-ol)
 ;;; test-ol.el ends here
-- 
2.20.1

>From 12726d18487298907ff63374120d17ee733383a7 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

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---
 lisp/ol.el | 53 ++---
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index d4bd0e40c..00f3bce5c 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -141,6 +141,15 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description.  A string is used as-is, a function is
+  called with two arguments: the full link text, and the
+  description generated by `org-insert-link'.  It should return
+  the description to use (this reflects the behaviour of
+  `org-link-make-description-function').
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1804,11 +1813,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 	 (origbuf (current-buffer))
@@ -1818,7 +1830,10 @@ don't allow to edit the default description."
 	 (desc region)
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry all-prefixes auto-desc)
+	 (all-prefixes (append (mapcar #'car abbrevs)
+			   (mapcar #'car org-link-abbrev-alist)
+			   (org-link-types)))
+ entry auto-desc type)
 (cond
  (link-location)		  ; specified by arg, just use it.
  ((org-in-regexp org-link-bracket-re 1)
@@ -1859,9 +1874,6 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
 	  (unless (pos-visible-in-window-p (point-max))
 	(org-fit-window-to-buffer))
 	  (and (window-live-p cw) (select-window cw
-  (setq all-prefixes (append (mapcar #'car abbrevs)
- (mapcar #'car org-link-abbrev-alist)
- (org-link-types)))
   (unwind-protect
 	  ;; Fake a link history, containing the stored links.
 	  (let ((org-link--history
@@ -1893,6 +1905,13 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
   (or entry (push link org-link--insert-history))
   (setq desc (o

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

[Max Nikulin]

On 21/06/2022 19:03, Hugo Heagren wrote:

+(defmacro test-ol-with-link-parameters-as (type parameters  body)

[...]

+  `(progn
+ (setq orig-parameters org-link-parameters)


I can easily miss something, but wouldn't it be enough to use let-binding

`(let ((org-link-parameters org-link-parameters))
   ,@body)

Otherwise it is better to use something like `condition-case' to restore 
original state even when some error is signaled.



+ (org-link-set-parameters ,type ,@parameters)
+ (let ((rtn (progn ,@body)))
+   (setq org-link-parameters orig-parameters)
+   rtn)))

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

[Robert Pluim]

> On Tue, 21 Jun 2022 13:03:32 +0100, Hugo Heagren  said:

Hugo> They're very nearly just right, but for some reason
Hugo> `test-ol-with-link-parameters-as' doesn't always reset the parameters
Hugo> correctly for me. However I have them set to originally, it sets
Hugo> `:default-description' to a lambda. This might be a quirk at my end
Hugo> though -- if someone else could have a look I'd be very grateful.

They way your macro is defined, it uses `orig-parameters' in a way
that it will leak into the global namespace. Better would be to do

(let ((orig-parameters org-link-parameters))
   ...
  (setq org-link-parameters orig-parameters)

Robert
-- 

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

[Hugo Heagren]

:default-description'
+  (should
+   (string=
+"notfoobar"
+(test-ol-with-link-parameters-as
+ "id" (:default-description "foobar")
+ (test-ol-insert-link-get-desc "id:foo-bar" "notfoobar")
+
 (provide 'test-ol)
 ;;; test-ol.el ends here
-- 
2.20.1

>From 437155c2394b5c9961ecc0af4e1e1a54b63416fe 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

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---
 lisp/ol.el | 47 +++
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index d4bd0e40c..1ab3a5f76 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -141,6 +141,15 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description.  A string is used as-is, a function is
+  called with two arguments: the full link text, and the
+  description generated by `org-insert-link'.  It should return
+  the description to use (this reflects the behaviour of
+  `org-link-make-description-function').
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1804,11 +1813,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 	 (origbuf (current-buffer))
@@ -1818,7 +1830,7 @@ don't allow to edit the default description."
 	 (desc region)
 	 (link link-location)
 	 (abbrevs org-link-abbrev-alist-local)
-	 entry all-prefixes auto-desc)
+	 entry all-prefixes auto-desc type)
 (cond
  (link-location)		  ; specified by arg, just use it.
  ((org-in-regexp org-link-bracket-re 1)
@@ -1893,6 +1905,13 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
   (or entry (push link org-link--insert-history))
   (setq desc (or desc (nth 1 entry)
 
+(setq type
+  (cond
+   ((string-match (rx (: string-start (submatch (eval `(or ,@all-prefixes))) ":")) link)
+(match-string 1 link))
+   ((file-name-absolute-p link) "file")
+   ((string-match "\\`\\.\\.?/" link) "file")))
+
 (when (funcall (if (equal complete-file '(64)) 'not 'identity)
 		   (not org-link-keep-stored-after-insertion))
   (setq org-stored-links (delq (assoc link org-stored-links)
@@ -1961,12 +1980,24 @@ Use TAB to complete link prefixes, then RET for type-specific completion support
   (let ((initial-input
 	 (cond
 	  (description)
-	  ((not org-link-make-description-function) desc)
+  (desc)
+  ((org-link-get-parameter type :default-description)
+   (let ((def (org-link-get-parameter type :default-description)))
+ (condition-case nil
+ (cond
+  ((stringp def) def)
+  ((functionp def)
+   (funcall def link desc)))
+   (error
+(message "Can't get link description from org link parameter `:default-description': %S"
+			 def)
+		(sit-for 2)
+nil
 	  (t (condition-case nil
 		 (funcall org-link-make-description-function link desc)
 		   (error
 		(message "Can't get link description from %S"
-			 (symbol-name org-link-make-description-function))
+		 org-link-make-description-function)
 		(sit-for 2)
 		nil))
 	(setq desc (if (called-interactively-p 'any)
-- 
2.20.1

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

[Ihor Radchenko]

Hugo Heagren  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

[PATCH v2] ol.el: add description format parameter to org-link-parameters

[Hugo Heagren]

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---

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.

Apologies if the subject formatting is not correct, I'm still getting
the hang of git-send-email.

Hugo

 lisp/ol.el | 43 ---
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 1b2bb9a9a..e74ef8dda 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -140,6 +140,15 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description. A string is used as-is, a function is
+  called with two arguments: the full link text, and the
+  description generated by `org-insert-linke'. It should return
+  the description to use (this reflects the behaviour of
+  `org-link-make-description-function').
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1761,11 +1770,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 (origbuf (current-buffer))
@@ -1775,7 +1787,7 @@ don't allow to edit the default description."
 (desc region)
 (link link-location)
 (abbrevs org-link-abbrev-alist-local)
-entry all-prefixes auto-desc)
+entry all-prefixes auto-desc type)
 (cond
  (link-location) ; specified by arg, just use it.
  ((org-in-regexp org-link-bracket-re 1)
@@ -1842,6 +1854,7 @@ Use TAB to complete link prefixes, then RET for 
type-specific completion support
  (and (equal ":" (substring link -1))
   (member (substring link 0 -1) all-prefixes)
   (setq link (substring link 0 -1
+  (setq type link)
  (setq link (with-current-buffer origbuf
   (org-link--try-special-completion link)
(set-window-configuration wcf)
@@ -1918,7 +1931,23 @@ Use TAB to complete link prefixes, then RET for 
type-specific completion support
   (let ((initial-input
 (cond
  (description)
- ((not org-link-make-description-function) desc)
+  (desc)
+  ((org-link-get-parameter
+type
+:default-description)
+   (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))
+ (sit-for 2)
+  nil)))
  (t (condition-case nil
 (funcall org-link-make-description-function link desc)
   (error
-- 
2.20.1

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

[Ihor Radchenko]

Hugo Heagren  writes:

> * ol.el (org-link-parameters): add parameter `:default-description', a
> string or a function.

LGTM in general. Note that I proposed (and never followed up on) a
similar patch in the past:
https://orgmode.org/list/87pnauvxht.fsf@localhost

I like that you introduced an option to set default description to
string, but I have several comments on the patch itself.


> -   (t (condition-case nil
> -  (funcall org-link-make-description-function link desc)
> +   (org-link-make-description-function
> +   (funcall org-link-make-description-function link desc))
> +  (t (error
> +   (message "Can't get link description from %S"
> +(symbol-name org-link-make-description-function))
> +   (sit-for 2)
> +   nil)

It looks like you removed condition-case used to catch errors on
org-link-make-description function. I am pretty sure that it is not
intentional.

Similar condition-case could also be used for the :default-description
function.

+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description. A string is used as-is, a function is
+  called with the full link text as the sole argument, and should
+  return a single string.

Why not two arguments like in org-link-make-description-function?

Best,
Ihor

[PATCH] ol.el: add description format parameter to org-link-parameters

[Hugo Heagren]

* ol.el (org-link-parameters): add parameter `:default-description', a
string or a function.
* (org-insert-link): if no description is provided (pre-existing or as
an argument), next option is to use the `:default-description' (if
non-nil) parameter to generate one.

Default descriptions are predictable within a link type, but because
link types are quite diverse, are NOT predictable across many types. A
type-parameter is thus a good place to store information on the
default description.
---
 lisp/ol.el | 49 +++--
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/lisp/ol.el b/lisp/ol.el
index 1b2bb9a9a..af0aaaf35 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -140,6 +140,13 @@ link.
   Function that inserts a link with completion.  The function
   takes one optional prefix argument.
 
+`:default-description'
+
+  String or function used as a default when prompting users for a
+  link's description. A string is used as-is, a function is
+  called with the full link text as the sole argument, and should
+  return a single string.
+
 `:display'
 
   Value for `invisible' text property on the hidden parts of the
@@ -1761,11 +1768,14 @@ prefix negates `org-link-keep-stored-after-insertion'.
 If the LINK-LOCATION parameter is non-nil, this value will be used as
 the link location instead of reading one interactively.
 
-If the DESCRIPTION parameter is non-nil, this value will be used as the
-default description.  Otherwise, if `org-link-make-description-function'
-is non-nil, this function will be called with the link target, and the
-result will be the default link description.  When called non-interactively,
-don't allow to edit the default description."
+If the DESCRIPTION parameter is non-nil, this value will be used
+as the default description.  If not, and the chosen link type has
+a non-nil `:default-description' parameter, that is used to
+generate a description as described in `org-link-parameters'
+docstring. Otherwise, if `org-link-make-description-function' is
+non-nil, this function will be called with the link target, and
+the result will be the default link description.  When called
+non-interactively, don't allow to edit the default description."
   (interactive "P")
   (let* ((wcf (current-window-configuration))
 (origbuf (current-buffer))
@@ -1775,7 +1785,7 @@ don't allow to edit the default description."
 (desc region)
 (link link-location)
 (abbrevs org-link-abbrev-alist-local)
-entry all-prefixes auto-desc)
+entry all-prefixes auto-desc type)
 (cond
  (link-location) ; specified by arg, just use it.
  ((org-in-regexp org-link-bracket-re 1)
@@ -1842,6 +1852,7 @@ Use TAB to complete link prefixes, then RET for 
type-specific completion support
  (and (equal ":" (substring link -1))
   (member (substring link 0 -1) all-prefixes)
   (setq link (substring link 0 -1
+  (setq type link)
  (setq link (with-current-buffer origbuf
   (org-link--try-special-completion link)
(set-window-configuration wcf)
@@ -1918,14 +1929,24 @@ Use TAB to complete link prefixes, then RET for 
type-specific completion support
   (let ((initial-input
 (cond
  (description)
- ((not org-link-make-description-function) desc)
- (t (condition-case nil
-(funcall org-link-make-description-function link desc)
-  (error
-   (message "Can't get link description from %S"
-(symbol-name org-link-make-description-function))
-   (sit-for 2)
-   nil))
+  (desc)
+  ((org-link-get-parameter
+type
+:default-description)
+   (let ((def (org-link-get-parameter
+   type
+   :default-description)))
+ (cond
+  ((stringp def) def)
+  ((functionp def)
+   (funcall def link)
+ (org-link-make-description-function
+   (funcall org-link-make-description-function link desc))
+  (t (error
+ (message "Can't get link description from %S"
+  (symbol-name org-link-make-description-function))
+ (sit-for 2)
+ nil)
(setq desc (if (called-interactively-p 'any)
   (read-string "Description: " initial-input)
 initial-input
-- 
2.20.1

ol.el: add description format parameter to org-link-parameters

[Hugo Heagren]

Suggested patch to add a new link parameter controlling how the
default description is generated. As explained in the commit, this
behaviour is often similar between links of the same type, but differs
between those types, so a parameter seems like a good place to specify
it.

I've tried to make `org-insert-link' behave sensibly with regard to
all the possible options -- hope it's alright.

Blue skies, Hugo