emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Add support for 'thing-at-point' to get URL at point
@ 2023-11-06 19:45 Jim Porter
  2023-11-06 19:56 ` Jim Porter
  2023-11-06 20:11 ` Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) Ihor Radchenko
  0 siblings, 2 replies; 10+ messages in thread
From: Jim Porter @ 2023-11-06 19:45 UTC (permalink / raw)
  To: emacs-orgmode

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

This is similar to Emacs bug#66752[1]. It would be nice if 
"(thing-at-point 'url)" would return the URL when point is over an Org 
link. With this, it's easier to write a function that copies (or browses 
to) the URL at point without coding so many special cases.

Attached is a patch with a regression test for it. Should this also get 
a NEWS entry?

[1] https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-10/msg01628.html

[-- Attachment #2: 0001-Add-support-for-thing-at-point-to-get-URL-at-point.patch --]
[-- Type: text/plain, Size: 2229 bytes --]

From 6bce84bd28253236eff8ef972ede7daf82f95a71 Mon Sep 17 00:00:00 2001
From: Jim Porter <itsjimporter@gmail.com>
Date: Mon, 6 Nov 2023 11:39:09 -0800
Subject: [PATCH] Add support for 'thing-at-point' to get URL at point

* lisp/org.el (thingatpt): Require.
(org--url-at-point): New function...
(org-mode): ... and add it to 'thing-at-point-provider-alist'.

* testing/lisp/test-org.el (test-org/thing-at-point/url): New test.
---
 lisp/org.el              | 10 ++++++++++
 testing/lisp/test-org.el | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 4eb6ad0ee..c7ecfc13a 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -81,6 +81,7 @@
 (require 'calendar)
 (require 'find-func)
 (require 'format-spec)
+(require 'thingatpt)
 
 (condition-case nil
     (load (concat (file-name-directory load-file-name)
@@ -4948,6 +4949,11 @@ The following commands are available:
             #'pcomplete-completions-at-point nil t)
   (setq-local buffer-face-mode-face 'org-default)
 
+  ;; `thing-at-point' support
+  (setq-local thing-at-point-provider-alist
+              (append thing-at-point-provider-alist
+                      '((url . org--url-at-point))))
+
   ;; If empty file that did not turn on Org mode automatically, make
   ;; it to.
   (when (and org-insert-mode-line-in-empty-file
@@ -8611,6 +8617,10 @@ there is one, return it."
 	   (setq link (nth (1- nth) links)))))
        (cons link end)))))
 
+(defun org--url-at-point ()
+  "`thing-at-point' provider function."
+  (org-element-property :raw-link (org-element-context)))
+
 ;;; File search
 
 (defun org-do-occur (regexp &optional cleanup)
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 21b850c03..2fe4477a3 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -3583,6 +3583,16 @@ Foo Bar
 	     (org-open-at-point))
 	 nil)))))
 
+\f
+;;; Thing at point
+
+(ert-deftest test-org/thing-at-point/url ()
+  "Test that `thing-at-point' returns the URL at point."
+  (should
+   (org-test-with-temp-text
+       "[[https://www.gnu.org/software/emacs/][GNU Emacs]]"
+     (string= (thing-at-point 'url) "https://www.gnu.org/software/emacs/"))))
+
 \f
 ;;; Node Properties
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add support for 'thing-at-point' to get URL at point
  2023-11-06 19:45 [PATCH] Add support for 'thing-at-point' to get URL at point Jim Porter
@ 2023-11-06 19:56 ` Jim Porter
  2023-11-06 20:11 ` Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) Ihor Radchenko
  1 sibling, 0 replies; 10+ messages in thread
From: Jim Porter @ 2023-11-06 19:56 UTC (permalink / raw)
  To: emacs-orgmode

On 11/6/2023 11:45 AM, Jim Porter wrote:
> This is similar to Emacs bug#66752[1]. It would be nice if 
> "(thing-at-point 'url)" would return the URL when point is over an Org 
> link. With this, it's easier to write a function that copies (or browses 
> to) the URL at point without coding so many special cases.

Actually, this code should probably be a bit more selective: is there a 
good way to tell when an Org link is an absolute URL, as opposed to some 
relative path or internal target?

Maybe we should check 'thing-at-point-uri-schemes'?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2023-11-06 19:45 [PATCH] Add support for 'thing-at-point' to get URL at point Jim Porter
  2023-11-06 19:56 ` Jim Porter
@ 2023-11-06 20:11 ` Ihor Radchenko
  2023-11-06 20:53   ` Jim Porter
  1 sibling, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2023-11-06 20:11 UTC (permalink / raw)
  To: Jim Porter, emacs-devel; +Cc: emacs-orgmode

[ Branching to emacs-devel for further input from Emacs devs ]

Jim Porter <jporterbugs@gmail.com> writes:

> This is similar to Emacs bug#66752[1]. It would be nice if 
> "(thing-at-point 'url)" would return the URL when point is over an Org 
> link. With this, it's easier to write a function that copies (or browses 
> to) the URL at point without coding so many special cases.
> ...
> +(defun org--url-at-point ()
> +  "`thing-at-point' provider function."
> +  (org-element-property :raw-link (org-element-context)))

Supporting thingatpt.el is certainly welcome. However, I have some
doubts about how mature thingatpt.el is.

In particular, I am concerned whether `thing-at-point-provider-alist' is
reliable enough in non-trivial scenarios like when given URL string is
not matching some generic URL regexp.

Looking into the source code of `bounds-of-thing-at-point', I see that
for standard "things" (like url),
`thing-at-point-bounds-of-url-at-point' is used unconditionally. In the
case of Org links, we may have something like [<point>[https://orgmode.org]]
that will not match default URL regexp as is. AFAIU, there is no
documented way to customize the behaviour of `bounds-of-thing-at-point'
and `forward-thing'.

I also have concerns about Org-specific part of the patch, but the above
is far more important, and we need to discuss it before starting to
consider anything for Org mode.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2023-11-06 20:11 ` Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) Ihor Radchenko
@ 2023-11-06 20:53   ` Jim Porter
  2024-02-05 15:07     ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Porter @ 2023-11-06 20:53 UTC (permalink / raw)
  To: Ihor Radchenko, emacs-devel; +Cc: emacs-orgmode

On 11/6/2023 12:11 PM, Ihor Radchenko wrote:
> [ Branching to emacs-devel for further input from Emacs devs ]
> 
> Jim Porter <jporterbugs@gmail.com> writes:
> 
>> This is similar to Emacs bug#66752[1]. It would be nice if
>> "(thing-at-point 'url)" would return the URL when point is over an Org
>> link. With this, it's easier to write a function that copies (or browses
>> to) the URL at point without coding so many special cases.
>> ...
>> +(defun org--url-at-point ()
>> +  "`thing-at-point' provider function."
>> +  (org-element-property :raw-link (org-element-context)))
> 
> Supporting thingatpt.el is certainly welcome. However, I have some
> doubts about how mature thingatpt.el is.
> 
> In particular, I am concerned whether `thing-at-point-provider-alist' is
> reliable enough in non-trivial scenarios like when given URL string is
> not matching some generic URL regexp.

The nice thing about 'thing-at-point-provider-alist' is that your 
provider has absolute control over what to return, so Org's URL provider 
could do whatever it wants. As far as I can tell, this code path 
completely avoids calling 'bounds-of-thing-at-point' ('botap'). However, 
it *would* call 'botap' if point wasn't on an Org link, since it would 
fall back to the last condition in 'thing-at-point'. Still, this is what 
happens today with no provider, so it's not really any worse than before...

Maybe it would make sense for 'thing-at-point' to have a "(catch 
'not-found ...)" form around the loop over 
'thing-at-point-provider-alist'. Then Org could definitively say, 
"There's no URL at point, no matter what anyone else says".

> Looking into the source code of `bounds-of-thing-at-point', I see that
> for standard "things" (like url),
> `thing-at-point-bounds-of-url-at-point' is used unconditionally. In the
> case of Org links, we may have something like [<point>[https://orgmode.org]]
> that will not match default URL regexp as is. AFAIU, there is no
> documented way to customize the behaviour of `bounds-of-thing-at-point'
> and `forward-thing'.

I think it would make sense to add some sort of 
'bounds-of-thing-at-point-provider-alist' (that's a mouthful!) that 
would let modes override the behavior of 'botap', but I don't think 
that's necessary for the narrower purpose of asking, "I want the value 
of THING at point, if any."

> I also have concerns about Org-specific part of the patch, but the above
> is far more important, and we need to discuss it before starting to
> consider anything for Org mode.

For better or worse, I mostly modeled this patch on how EWW integrates 
with thing-at-point, since that's the only place I saw in the Emacs tree 
that did this already.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2023-11-06 20:53   ` Jim Porter
@ 2024-02-05 15:07     ` Ihor Radchenko
  2024-02-05 22:44       ` Jim Porter
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2024-02-05 15:07 UTC (permalink / raw)
  To: Jim Porter; +Cc: emacs-devel, emacs-orgmode

Jim Porter <jporterbugs@gmail.com> writes:

>> Looking into the source code of `bounds-of-thing-at-point', I see that
>> for standard "things" (like url),
>> `thing-at-point-bounds-of-url-at-point' is used unconditionally. In the
>> case of Org links, we may have something like [<point>[https://orgmode.org]]
>> that will not match default URL regexp as is. AFAIU, there is no
>> documented way to customize the behaviour of `bounds-of-thing-at-point'
>> and `forward-thing'.
>
> I think it would make sense to add some sort of 
> 'bounds-of-thing-at-point-provider-alist' (that's a mouthful!) that 
> would let modes override the behavior of 'botap', but I don't think 
> that's necessary for the narrower purpose of asking, "I want the value 
> of THING at point, if any."

It would make sense to add a number of alists:
- bounds-of-thing-at-point-provider-alist
- same for 'forward-op, 'beginning-op, 'end-op.

After Emacs have those, we can add Org mode support.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2024-02-05 15:07     ` Ihor Radchenko
@ 2024-02-05 22:44       ` Jim Porter
  2024-02-05 22:56         ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Porter @ 2024-02-05 22:44 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-devel, emacs-orgmode

On 2/5/2024 7:07 AM, Ihor Radchenko wrote:
> It would make sense to add a number of alists:
> - bounds-of-thing-at-point-provider-alist
> - same for 'forward-op, 'beginning-op, 'end-op.
> 
> After Emacs have those, we can add Org mode support.

That sounds reasonable enough to me; does anyone else have opinions on 
this? Otherwise, I'll get to work on a patch (though probably not for a 
couple weeks).


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2024-02-05 22:44       ` Jim Porter
@ 2024-02-05 22:56         ` Ihor Radchenko
  2024-02-06 12:26           ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2024-02-05 22:56 UTC (permalink / raw)
  To: Jim Porter, stefankangas, Eli Zaretskii; +Cc: emacs-devel, emacs-orgmode

Jim Porter <jporterbugs@gmail.com> writes:

> On 2/5/2024 7:07 AM, Ihor Radchenko wrote:
>> It would make sense to add a number of alists:
>> - bounds-of-thing-at-point-provider-alist
>> - same for 'forward-op, 'beginning-op, 'end-op.
>> 
>> After Emacs have those, we can add Org mode support.
>
> That sounds reasonable enough to me; does anyone else have opinions on 
> this? Otherwise, I'll get to work on a patch (though probably not for a 
> couple weeks).

CCing Stefan and Eli.
Please, let us know if the above is something not wanted upstream.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2024-02-05 22:56         ` Ihor Radchenko
@ 2024-02-06 12:26           ` Eli Zaretskii
  2024-02-06 12:38             ` Ihor Radchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2024-02-06 12:26 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: jporterbugs, stefankangas, emacs-devel, emacs-orgmode

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: emacs-devel@gnu.org, emacs-orgmode@gnu.org
> Date: Mon, 05 Feb 2024 22:56:05 +0000
> 
> Jim Porter <jporterbugs@gmail.com> writes:
> 
> > On 2/5/2024 7:07 AM, Ihor Radchenko wrote:
> >> It would make sense to add a number of alists:
> >> - bounds-of-thing-at-point-provider-alist
> >> - same for 'forward-op, 'beginning-op, 'end-op.
> >> 
> >> After Emacs have those, we can add Org mode support.
> >
> > That sounds reasonable enough to me; does anyone else have opinions on 
> > this? Otherwise, I'll get to work on a patch (though probably not for a 
> > couple weeks).
> 
> CCing Stefan and Eli.
> Please, let us know if the above is something not wanted upstream.

I think we do want to allow extending of this, but doesn't
thingatpt.el already provide such capabilities?  For example, I see
this in bounds-of-thing-at-point:

  (defun bounds-of-thing-at-point (thing)
    "Determine the start and end buffer locations for the THING at point.
  THING should be a symbol specifying a type of syntactic entity.
  Possibilities include `symbol', `list', `sexp', `defun', `number',
  `filename', `url', `email', `uuid', `word', `sentence', `whitespace',
  `line', and `page'.

  See the file `thingatpt.el' for documentation on how to define a
  valid THING.

  Return a cons cell (START . END) giving the start and end
  positions of the thing found."
    (cond
     ((get thing 'bounds-of-thing-at-point)  <<<<<<<<<<<<<<<<<<<<<<<<
      (funcall (get thing 'bounds-of-thing-at-point)))

Doesn't this provide the extension capabilities you are looking for?
If not, why not?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2024-02-06 12:26           ` Eli Zaretskii
@ 2024-02-06 12:38             ` Ihor Radchenko
  2024-02-06 12:47               ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Ihor Radchenko @ 2024-02-06 12:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jporterbugs, stefankangas, emacs-devel, emacs-orgmode

Eli Zaretskii <eliz@gnu.org> writes:

> I think we do want to allow extending of this, but doesn't
> thingatpt.el already provide such capabilities?  For example, I see
> this in bounds-of-thing-at-point:
> ...
>     (cond
>      ((get thing 'bounds-of-thing-at-point)  <<<<<<<<<<<<<<<<<<<<<<<<
>       (funcall (get thing 'bounds-of-thing-at-point)))
>
> Doesn't this provide the extension capabilities you are looking for?
> If not, why not?

Unlike `thing-at-point-provider-alist', which can be buffer-local,
symbol property is always global and setting it would override other
thing providers.

Note how `thing-at-point' uses

(cond
          ((let ((alist thing-at-point-provider-alist)
                 elt result)
             (while (and alist (null result))
               (setq elt (car alist)
                     alist (cdr alist))
               (and (eq (car elt) thing)
                    (setq result (funcall (cdr elt)))))
             result))
          ((get thing 'thing-at-point)
           (funcall (get thing 'thing-at-point)))

checking `thing-at-point-provider-alist' and only then falling back to
`get'. What I am proposing is to add the equivalent alists for other
operators used by thingatpt.el.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
  2024-02-06 12:38             ` Ihor Radchenko
@ 2024-02-06 12:47               ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2024-02-06 12:47 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: jporterbugs, stefankangas, emacs-devel, emacs-orgmode

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: jporterbugs@gmail.com, stefankangas@gmail.com, emacs-devel@gnu.org,
>  emacs-orgmode@gnu.org
> Date: Tue, 06 Feb 2024 12:38:19 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I think we do want to allow extending of this, but doesn't
> > thingatpt.el already provide such capabilities?  For example, I see
> > this in bounds-of-thing-at-point:
> > ...
> >     (cond
> >      ((get thing 'bounds-of-thing-at-point)  <<<<<<<<<<<<<<<<<<<<<<<<
> >       (funcall (get thing 'bounds-of-thing-at-point)))
> >
> > Doesn't this provide the extension capabilities you are looking for?
> > If not, why not?
> 
> Unlike `thing-at-point-provider-alist', which can be buffer-local,
> symbol property is always global and setting it would override other
> thing providers.
> 
> Note how `thing-at-point' uses
> 
> (cond
>           ((let ((alist thing-at-point-provider-alist)
>                  elt result)
>              (while (and alist (null result))
>                (setq elt (car alist)
>                      alist (cdr alist))
>                (and (eq (car elt) thing)
>                     (setq result (funcall (cdr elt)))))
>              result))
>           ((get thing 'thing-at-point)
>            (funcall (get thing 'thing-at-point)))
> 
> checking `thing-at-point-provider-alist' and only then falling back to
> `get'. What I am proposing is to add the equivalent alists for other
> operators used by thingatpt.el.

I guess it's fine, then.

But we probably should have such alists in all the other thingatpt
methods as well.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-02-06 12:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-06 19:45 [PATCH] Add support for 'thing-at-point' to get URL at point Jim Porter
2023-11-06 19:56 ` Jim Porter
2023-11-06 20:11 ` Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) Ihor Radchenko
2023-11-06 20:53   ` Jim Porter
2024-02-05 15:07     ` Ihor Radchenko
2024-02-05 22:44       ` Jim Porter
2024-02-05 22:56         ` Ihor Radchenko
2024-02-06 12:26           ` Eli Zaretskii
2024-02-06 12:38             ` Ihor Radchenko
2024-02-06 12:47               ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).