Re: [PATCH] Add support for shortdoc link type

[Max Nikulin <manikulin@gmail.com>]

On 09/05/2024 04:26, Bruno Cardoso wrote:
> On 2024-05-08, 18:20 +0700, Max Nikulin wrote:
> 
>> I have not managed to get all known groups with `shortdoc-display-group'
>> in Emacs-28. Is it a feature of `shortdoc' in Emacs-29?
> 
> In Emacs-29 `shortdoc' is an alias for `shortdoc-display-group'.
> Interactvely, it prompts the user for a group from `shortdoc--groups'
> (that's what I meant by "all known groups").

Thank you for the explanation. Reading "lists all" I expected something 
like list of all info manuals in the case of M-x info RET when there are 
no info buffers yet.

>> "\\`\\([^:]+\\)\\(?:::\\(.+\\)\\'\\)?"
> 
> `shortdoc-display-group' already handles empty group or function.
> Replacing "*" by "+" will throw an "Args out of range" error in the
> corner case you mentioned ("shortdoc:::file"). So I think it's fine to
> keep "*" there.

Out of range error happens because of `string-match' is called despite 
`match-string' failure and match groups from some earlier regexp are 
applied. Generally `match-string' should be inside `if', `when', `and', 
etc., however I agree that the regexp with stars always succeeds, 
perhaps really matching nothing. That is why I do not insist.

The following garbage in - garbage out case is a bit confusing:
<shortdoc::*file>
=> "No such documentation group "

> ol.el: Add support for `shortdoc' link type

Bruno, has you signed the copyright form? This patch is above the 
TINYCHANGE limit.
<https://orgmode.org/worg/org-contribute.html#copyright>

It will be a bit less work for the maintainer if you attach result of 
"git format-patch" command. It adds email-like headers allowing to 
properly set commit author.

> * lisp/ol.el: Add support for storing and inserting links to `shortdoc'
> documentation groups for Emacs Lisp functions.

List of new functions is missed:

* lisp/ol.el (org-link--open-shortdoc org-link--store-shortdoc)
(org-link--complete-shortdoc): Add support...

> * doc/org-manual.org (External Links): Add shortdoc link type
> documentation.
> 
> * etc/ORG-NEWS (=ol.el=: Support for =shortdoc= link type): Document
> the new feature.

Empty line is separator before free-form comments, so list of formal 
changes should be dense.

> +      (condition-case nil
> +          (progn
> +            (shortdoc-display-group group fn)
> +            (and str (not fn) (search-forward str nil t)))
> +        (user-error "Unknown shortdoc group: %s" group))))

Sorry, it does not work so. It means catch `user-error' however 
`shortdoc-display-group' signals `error'. Try to enable debug-on-error 
and to open a broken link. Either `condition-case' should be dropped or 
proper signal should be captured.