Re: [PATCH] Add support for shortdoc link type

[Max Nikulin]

On 05/05/2024 00:33, Bruno Cardoso wrote:
> I updated the patch to use the optional function argument instead.

Thanks

> +++ b/doc/org-manual.org
> @@ -3372,6 +3372,10 @@ Here is the full set of built-in link types:
>  
>    Execute a shell command upon activation.
>  
> +- =shortdoc= ::
> +
> +Link to short documentation summary for a function group.  For more 
> information, see [[info:emacs#Name Help][Help by Command or Variable Name]].

I would consider explicit mention that it is related to Emacs Lisp and 
perhaps that the origin of its name is the ~shortdoc-display-group~ user 
command. <info:elisp#Documentation Groups> might be a better link.

This line is rather long though I do not have a link where formatting 
conventions are documented.

> +++ b/etc/ORG-NEWS
> @@ -1369,6 +1369,10 @@ place the entry in the ~Misc~ category if 
> ~TEXINFO_DIR_CATEGORY~ is missing.
>  =TEXINFO_DIR_TITLE= is renamed to =TEXINFO_DIR_NAME=.
>  The old name is obsolete.
>  
> +*** =ol.el=: Support for =shortdoc= link type
> +
> +Add support for storing and inserting links to =shortdoc= groups.

Again I am unsure that it is clear to all readers.

> +++ b/lisp/ol.el
> @@ -1582,6 +1582,41 @@ PATH is a symbol name, as a string."
>                           :follow #'org-link--open-help
>                           :store #'org-link--store-help)
>  
> +;;;; "shortdoc" link type
> +(when (version< "27" emacs-version)

Is it correct?

2a7488d42d8 2020-10-11 05:51:16 +0200 Lars Ingebrigtsen: Add support for 
displaying short documentation for function groups

git tag --contains 2a7488d42d8
emacs-28.0.90

> +  (defun org-link--open-shortdoc (path _)
> +    "Open a \"shortdoc\" type link.
> +PATH is a group name or \"group::#function\"."
> +    (string-match "\\`\\([^#:]*\\)\\(::#?\\(.*\\)\\)?\\'" path)

I think, it is enough to use [^:] since next group is started from "::". 
You may use use non-capturing group \\(?:::# ...\\) (requires update of 
next index below) unless you are going add search code back. I am unsure 
concerning your intentions. You dropped search code, but "#" after "::" 
is still optional.

> +    (let ((group (match-string 1 path))
> +          (fn (match-string 3 path)))
> +      (condition-case nil
> +          (shortdoc-display-group group (intern-soft fn))
> +        (error (message "Unknown shortdoc group: %s" group)))))

I do not have strong opinion what is better here: `user-error' (used in 
earlier revisions) or `error'. However I do not see the point of 
capturing original error and signalling another one if it is not 
`user-error'.