[BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[Magnus Therning]

[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]

Remember to cover the basics, that is, what you expected to happen 
and
what in fact did happen.  You don't know how to make a good 
report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

The documenation on org-create-file-search-functions contains this 
piece of text

  A function in this hook may also use setq to set the variable 
  description to provide a suggestion for the descriptive text to 
  be used for this link when it gets inserted into an Org buffer 
  with org-insert-link.

This doesn't seem to be true though. I really would love for there 
to be a way to influence the link description, but no matter what 
the documentation should be corrected.

For some more info look at 
https://www.reddit.com/r/orgmode/comments/xmvtsy/orgcreatefilesearchfunctions_and_description/

Emacs  : GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ 
Version 3.24.34, cairo version 1.17.6)
 of 2022-09-21
Package: Org mode version 9.5.5 (release_9.5.5 @ 
/usr/share/emacs/29.0.50/lisp/org/)

/M

--
Magnus Therning                   OpenPGP: 0x927912051716CE39
email: magnus@therning.org
@magthe@mastodon.technology       http://magnus.therning.org/

You can't depend on your judgement when your imagination is out of 
focus.
     — Mark Twain

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[Ihor Radchenko]

Magnus Therning <magnus@therning.org> writes:

> The documenation on org-create-file-search-functions contains this 
> piece of text
>
>   A function in this hook may also use setq to set the variable 
>   description to provide a suggestion for the descriptive text to 
>   be used for this link when it gets inserted into an Org buffer 
>   with org-insert-link.
>
> This doesn't seem to be true though. I really would love for there 
> to be a way to influence the link description, but no matter what 
> the documentation should be corrected.
>
> For some more info look at 
> https://www.reddit.com/r/orgmode/comments/xmvtsy/orgcreatefilesearchfunctions_and_description/

Thanks for reporting!
Confirmed.

This is a regression after we switched to lexical binding, and after we
removed the apparently uninitialized `description' variable from
let-binding in `org-store-link'.

    a85b4cc5c64b1eb6b97f5d7843dd54eb0632ef91
    Author:     Stefan Monnier <monnier@iro.umontreal.ca>
    AuthorDate: Thu Mar 11 13:07:37 2021 -0500
    Commit:     Kyle Meyer <kyle@kyleam.com>
    CommitDate: Sun Mar 14 17:54:17 2021 -0400
    
    Backport commit 7d0dc3183 from Emacs
    
    ...
    * lisp/ol.el (org-store-link):
    Remove always-nil variable `description`.

In order to make things work now with lexical binding, we need to create
a top-level defvar in ol.el, which is not nice since `description'
variable name is a bad name to define.

Interestingly, this issue has not been noticed for a long time and not
many users are probably making use of it.

Thus, we may as well take this opportunity and allow
`org-create-file-search-functions' to return a cons cell
("search term" . "description"). This will not require unintuitive
global variables.

WDYT?

-- 
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: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[Magnus Therning]

[-- Attachment #1: Type: text/plain, Size: 2330 bytes --]


Ihor Radchenko <yantar92@gmail.com> writes:

> Magnus Therning <magnus@therning.org> writes:
>
>> The documenation on org-create-file-search-functions contains 
>> this
>> piece of text
>>
>>   A function in this hook may also use setq to set the variable
>>   description to provide a suggestion for the descriptive text 
>>   to
>>   be used for this link when it gets inserted into an Org 
>>   buffer
>>   with org-insert-link.
>>
>> This doesn't seem to be true though. I really would love for 
>> there
>> to be a way to influence the link description, but no matter 
>> what
>> the documentation should be corrected.
>>
>> For some more info look at
>> https://www.reddit.com/r/orgmode/comments/xmvtsy/orgcreatefilesearchfunctions_and_description/
>
> Thanks for reporting!
> Confirmed.
>
> This is a regression after we switched to lexical binding, and 
> after we
> removed the apparently uninitialized `description' variable from
> let-binding in `org-store-link'.
>
>     a85b4cc5c64b1eb6b97f5d7843dd54eb0632ef91
>     Author:     Stefan Monnier <monnier@iro.umontreal.ca>
>     AuthorDate: Thu Mar 11 13:07:37 2021 -0500
>     Commit:     Kyle Meyer <kyle@kyleam.com>
>     CommitDate: Sun Mar 14 17:54:17 2021 -0400
>
>     Backport commit 7d0dc3183 from Emacs
>
>     ...
>     * lisp/ol.el (org-store-link):
>     Remove always-nil variable `description`.
>
> In order to make things work now with lexical binding, we need 
> to create
> a top-level defvar in ol.el, which is not nice since 
> `description'
> variable name is a bad name to define.
>
> Interestingly, this issue has not been noticed for a long time 
> and not
> many users are probably making use of it.
>
> Thus, we may as well take this opportunity and allow
> `org-create-file-search-functions' to return a cons cell
> ("search term" . "description"). This will not require 
> unintuitive
> global variables.
>
> WDYT?

I think returning a cons cell would be a very nice solution.

/M

--
Magnus Therning                   OpenPGP: 0x927912051716CE39
email: magnus@therning.org
@magthe@mastodon.technology       http://magnus.therning.org/

Never argue with stupid people, they will drag you down to their 
level
and then beat you with experience.
     ― Mark Twain

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[Max Nikulin]

On 26/09/2022 19:12, Ihor Radchenko wrote:
> Magnus Therning writes:
> 
>> The documenation on org-create-file-search-functions contains this
>> piece of text
>>
>>    A function in this hook may also use setq to set the variable
>>    description to provide a suggestion for the descriptive text to
>>    be used for this link when it gets inserted into an Org buffer
>>    with org-insert-link.
>>
>> This doesn't seem to be true though. I really would love for there
>> to be a way to influence the link description, but no matter what
>> the documentation should be corrected.
> 
> Thus, we may as well take this opportunity and allow
> `org-create-file-search-functions' to return a cons cell
> ("search term" . "description"). This will not require unintuitive
> global variables.

There is the `org-store-link-plist' variable used by :store functions 
from `org-link-parameters' but not by the 
`org-create-file-search-functions' hook. Maybe it is enough to add

    (setq desc (plist-get org-store-link-plist :description))

after calling of the hook.

For a while, I hope, it is possible to create a workaround using

   (org-link-set-parameters
    "file"
    :store ...)

Re: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

>> Thus, we may as well take this opportunity and allow
>> `org-create-file-search-functions' to return a cons cell
>> ("search term" . "description"). This will not require unintuitive
>> global variables.
>
> There is the `org-store-link-plist' variable used by :store functions 
> from `org-link-parameters' but not by the 
> `org-create-file-search-functions' hook. Maybe it is enough to add
>
>     (setq desc (plist-get org-store-link-plist :description))
>
> after calling of the hook.

This makes sense. Instead of making `org-create-file-search-functions'
return a cons cell, we can use the same scheme as in
`org-store-link-functions' where the functions can call
`org-link-store-props' inside to set e.g. the description.

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

[BUG] org-store-link-functions advertizes that the first non-nil return value is used, but it is not how org-store-link handles it (was: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)])

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> There is the `org-store-link-plist' variable used by :store functions 
> from `org-link-parameters' but not by the 
> `org-create-file-search-functions' hook. Maybe it is enough to add

Reading through `org-store-link-functions' docstring and
`org-store-link' code, I noticed that `org-store-link-functions'
promises the following:

    Each function will be called in turn until one returns a non-nil
    value.

Yet, `org-store-link' does the following:

(dolist (f (org-store-link-functions))
		 (when (funcall f)
		   (push (cons f (copy-sequence org-store-link-plist))
			 results-alist)))

(pcase results-alist
                  ...
		  ;; Reinstate link plist associated to the chosen
		  ;; function.
		  (apply #'org-link-store-props
			 (cdr (assoc-string
			       (completing-read
                                (format "Store link with (default %s): " name)
                                (mapcar #'car results-alist)
                                nil t nil nil (symbol-name name))
			       results-alist)))
		  t))

That is, all the store link functions are actually being executed, not
until first non-nil return value. If multiple non-nil values are
returned, an interactive query is displayed to the user.

---

The actual behaviour is indeed nice, but I am wondering how it is going
to work in non-interactive case.

Thoughts?

-- 
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: [BUG] org-store-link-functions advertizes that the first non-nil return value is used, but it is not how org-store-link handles it

[Max Nikulin]

On 27/09/2022 08:40, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> There is the `org-store-link-plist' variable used by :store functions
>> from `org-link-parameters' but not by the
>> `org-create-file-search-functions' hook. Maybe it is enough to add
> 
> Reading through `org-store-link-functions' docstring and
> `org-store-link' code, I noticed that `org-store-link-functions'
> promises the following:
...
> That is, all the store link functions are actually being executed, not
> until first non-nil return value. If multiple non-nil values are
> returned, an interactive query is displayed to the user.
> 
> The actual behaviour is indeed nice, but I am wondering how it is going
> to work in non-interactive case.

There are more inconsistencies. For a heading in an Org file 2 links may 
be created: title search and #custom_id. Only one link is removed by 
`org-insert-link'. Only one of them is checked if it has been stored 
earlier. Usually after several store+insert actions I have a list of 
stored links that I will never use. I am unsure what is better, to 
prompt user when link is saved, or store all options with some group ID 
and remain other links when one item from the group is inserted.

I suppose, `org-store-link' should be split into smaller building blocks 
to allow experiments with alternative implementations and strategies to 
select from several variants.

I was wrong suggestion that changing of :store property of "file" links 
may be a workaround for original issue. It is easy to store line number 
instead of search text, but there are a lot of code to detect headings, 
#custom_id, <<target>>, etc. for "file" links and this code can not be 
easily reused.

Re: [BUG] org-store-link-functions advertizes that the first non-nil return value is used, but it is not how org-store-link handles it

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> There are more inconsistencies. For a heading in an Org file 2 links may 
> be created: title search and #custom_id. Only one link is removed by 
> `org-insert-link'. Only one of them is checked if it has been stored 
> earlier. Usually after several store+insert actions I have a list of 
> stored links that I will never use. I am unsure what is better, to 
> prompt user when link is saved, or store all options with some group ID 
> and remain other links when one item from the group is inserted.

Would you mind reporting this and MWE in a new thread?
It will be easier to track then.

> I suppose, `org-store-link' should be split into smaller building blocks 
> to allow experiments with alternative implementations and strategies to 
> select from several variants.

+1

-- 
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: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)]

[Ihor Radchenko]

Ihor Radchenko <yantar92@gmail.com> writes:

> This makes sense. Instead of making `org-create-file-search-functions'
> return a cons cell, we can use the same scheme as in
> `org-store-link-functions' where the functions can call
> `org-link-store-props' inside to set e.g. the description.

Fixed, on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=96b312926

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

Re: [BUG] org-store-link-functions advertizes that the first non-nil return value is used, but it is not how org-store-link handles it (was: [BUG] org-create-file-search-functions and description [9.5.5 (release_9.5.5 @ /usr/share/emacs/29.0.50/lisp/org/)])

[Ihor Radchenko]

Ihor Radchenko <yantar92@gmail.com> writes:

> Max Nikulin <manikulin@gmail.com> writes:
>
>> There is the `org-store-link-plist' variable used by :store functions 
>> from `org-link-parameters' but not by the 
>> `org-create-file-search-functions' hook. Maybe it is enough to add
>
> Reading through `org-store-link-functions' docstring and
> `org-store-link' code, I noticed that `org-store-link-functions'
> promises the following:
>
>     Each function will be called in turn until one returns a non-nil
>     value.
>
> Yet, `org-store-link' does the following:
>
> (dolist (f (org-store-link-functions))
> 		 (when (funcall f)
> 		   (push (cons f (copy-sequence org-store-link-plist))
> 			 results-alist)))
> ...
> That is, all the store link functions are actually being executed, not
> until first non-nil return value. If multiple non-nil values are
> returned, an interactive query is displayed to the user.

I updated the docstring to reflect the ground reality for now.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c6849a2bc

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