Re: [O] links-9.0 v3

[Nicolas Goaziou]

John Kitchin <address@hidden> writes:

>> Here is the gotcha. `type' is "file", not "file+sys" or "file+emacs",
>> so, when checking `dedicated-function' first, we cannot tell the
>> difference between "file+sys" and "file+emacs".
>
> I don't follow this. Why can't these be three types?

The type is "file". "sys" or "emacs" are indications about how to follow
them. "docview" is also an indication, but it really points to a "file".

> They define 3 different follow actions right? Those are basically
> equal to pressing RET, C-u RET and C-u C-u RET on a link. We could
> just define three :follow functions, and one :export function for
> them.

It doesn't mean they cannot have an entry in `org-link-parameters'.
Actually they should.

However `org-link-protocols' keys are applications, not types. So

  (org-link-get-parameter type :follow)

is not a drop-in replacement for

  (nth 1 (assoc app org-link-protocols))

> With the patches I sent, the three types actually work as they used to,
> e.g. file:some.pptx opens the powerpoint file in emacs (not convenient
> ;), file+sys:some.pptx opens it in powerpoint. This seems to be because
> org-element-context (via org-element-link-parser) sees file+sys and
> file+emacs as a file type.

Which is correct.

> Overall, this is an inconsistency to me. On one hand, we need file+sys
> and file+emacs in org-link-parameters so that they are built into the
> link regexps (or they would have to be hard-coded in the function that
> makes the regexp.

[...]

> On the other hand, the org-open-at-point
> function bypasses the link properties, so it is not possible to change
> the :follow function for file+sys and file+emacs.

Of course it is possible. I suggested two solutions already.

>> One solution is to swap the logic order. First, if app is non-nil, we
>> use it. If it isn't, we look after `dedicated-function'.
>>
>> Another solution is to add an optional parameter to the signature of
>> the :follow function, which would be the "app" (e.g. "emacs", "sys",
>> "docview"...) to use. I tend to think this solution is slightly better,
>> since it doesn't require to hard-code logic in `org-open-at-point'.
>>
>> WDYT?
>
> I am not crazy about this solution,

Which one? I suggested two of them.

> since it only applies to one type of link,

Does it? Every type can benefit from this change, i.e. instead of
calling follow function with a single argument, it is called with two,
the second one being the application (e.g., "sys", "emacs"...) or nil.

> and I can't see how to use it for other follow functions. It would be
> better IMO to define :follow functions maybe like this:
>
> "file" :follow #'org-open-at-point
> "file+sys" :follow (lambda (_) (org-open-at-point '(4)))
> "file+emacs" :follow (lambda (_) (org-open-at-point '(16)))
>
> and make them be honored in org-open-at-point. Then we could eliminate
> the logic code in org-open-at-point.

You may be misunderstanding the problem. 

The issue is that `org-open-at-point', at the moment, cannot call
the :follow function for "file+emacs" or "file+sys" since the common
type is "file", even if `org-link-parameters' distinguish them. IOW the
first :follow function would always be called.

Also, `org-open-at-point' shouldn't be part of a follow function, since
`org-open-at-point' calls follow functions. It can call `org-open-file',
tho, as currently done in `org-open-at-point'.

Actually, I can think of a third solution, which may well follow the
path of least resistance. Instead of calling

  (org-link-get-parameter type :follow)

we would call

  (org-link-get-parameter (if app (concat type "+" app) type) :follow)

and get the appropriate follow function. This solution is local to
`org-open-at-point', but I don't think other places need :follow
function.

>>>    (let ((data (assoc type org-link-parameters)))
>>> -    (if data
>>> -   (cl-loop for (key val) on parameters by #'cddr
>>> -            do
>>> -            (setf (cl-getf (cdr data) key)
>>> -                  val))
>>> +    (if data (setcdr data (org-combine-plists (cdr data) parameters))
>>>        (push (cons type parameters) org-link-parameters)
>>>        (org-make-link-regexps)
>>>        (org-element-update-syntax))))
>>
>> This change can be merged with `org-link-set-parameters' definition.
>
> I am not sure how to do that. It is like a hunk in one commit that I
> want to move to another commit.

I would edit the commit defining `org-link-set-parameters' and install
that change there. Then, upon rebasing, I would make sure this change is
preserved.

>>> +(defcustom org-link-parameters
>>> +  '(("http") ("https") ("ftp") ("mailto")
>>> +    ("file" :complete 'org-file-complete-link)
>>> +    ("file+emacs") ("file+sys")
>>> +    ("news") ("shell") ("elisp")
>>> +    ("doi") ("message") ("help"))
>>
>> See above about "file+emacs" and "file+sys", which are not valid types.
>
> Those either need to be here for link regexps, or hard-coded somewhere
> else though.

You're right, they need to be here, but there is still an issue.

>Speaking of which, should coderef be a link type, so it can
> be removed as a hard-coded string that I referenced above?

No it cannot. "coderef" is not a valid link type, since there is no
[[coderef:something]]. It shouldn't belong to the link regexp.

Regards,