* RE: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point)
@ 2024-05-08 16:09 Pedro Andres Aranda Gutierrez
2024-05-08 18:14 ` Drew Adams
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Andres Aranda Gutierrez @ 2024-05-08 16:09 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 875 bytes --]
Jim wrote:
> At the risk of veering off-topic (I mainly care about adding
> 'bounds-of-thing-at-point-provider-alist' and
> 'forward-thing-provider-alist'), would adding a new optional STRICT
> argument to 'thing-at-point' and friends be an ok resolution for
> everyone? This argument would enable Drew's proposed behavior. That way,
> users get all the nice behavior by default just like today, and
> programmers who require strict correctness in their code also have an
> option.
1+ if STRICT means thing starting at point and without it we may need to
look for the beginning of the thing (lazy point setting)....
/PA
--
Fragen sind nicht da, um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler
Headaches with a Juju log:
unit-basic-16: 09:17:36 WARNING juju.worker.uniter.operation we should run
a leader-deposed hook here, but we can't yet
[-- Attachment #2: Type: text/html, Size: 1309 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-08 16:09 Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) Pedro Andres Aranda Gutierrez @ 2024-05-08 18:14 ` Drew Adams 2024-05-09 5:46 ` Pedro Andres Aranda Gutierrez 2024-05-15 8:06 ` Jean Louis 0 siblings, 2 replies; 9+ messages in thread From: Drew Adams @ 2024-05-08 18:14 UTC (permalink / raw) To: Pedro Andres Aranda Gutierrez, emacs-devel >> At the risk of veering off-topic (I mainly care about adding >> 'bounds-of-thing-at-point-provider-alist' and >> 'forward-thing-provider-alist'), would adding a new optional STRICT >> argument to 'thing-at-point' and friends be an ok resolution for >> everyone? This argument would enable Drew's proposed behavior. That way, >> users get all the nice behavior by default just like today, and >> programmers who require strict correctness in their code also have an >> option. > > 1+ if STRICT means thing starting at point > and without it we may need to look for the > beginning of the thing (lazy point setting).... Why "starting" at point? STRICT shouldn't mean checking whether a THING starts at point. The "strict" behavior that's needed is checking _whether_ there is a THING at point, that is, checking whether the char at point (which really means just after point) is on/inside a THING. That's for `bounds-of-thing-at-point' etc. For thing-at-point etc., IF there's a THING at point then that THING is the non-nil value returned. In terms of implementation, I suggest you take a look at the code in thingatpt+.el. Look at functions `tap-bounds-of-thing-at-point' and `tap-thing-at-point'. Just remove the `tap-' prefix for code that DTRT. The file Commentary explains things in detail. My code adds an optional arg SYNTAX-TABLE, but you need not bother with that. Elisp now has `with-syntax-table', which can be used as a workaround if there's no such argument. In the end, `bounds-of-thing-at-point' and `thing-at-point' should return nil if there's no THING at the char at point. For "lax" behavior that corresponds to the current vanilla behavior, if the strict behavior would return nil then they return what the strict behavior would return at (1- point). But really such a lax behavior is pretty lame. What's needed, for trying to grab a THING near point is not just checking backward one char but checking backward, forward, up, and down N chars, where N determines what "near" means. thingatpt+.el provides two user options, `tap-near-point-x-distance' and `tap-near-point-y-distance', the max number of chars from point to check for a THING (for X: left and right, for Y: up and down). Setting the Y value to zero constrains search to the same line as point. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-08 18:14 ` Drew Adams @ 2024-05-09 5:46 ` Pedro Andres Aranda Gutierrez 2024-05-15 8:06 ` Jean Louis 1 sibling, 0 replies; 9+ messages in thread From: Pedro Andres Aranda Gutierrez @ 2024-05-09 5:46 UTC (permalink / raw) To: Drew Adams, Org Mode List [-- Attachment #1: Type: text/plain, Size: 3231 bytes --] H, Drew Hmmm... thanks for the clarification. That's going to be extremely helpful once I'm back on track after the "grading season" ;-) All this started, at least for me, when I was trying to write a simple interactive function to copy the contents of the cell the point is in to the kill-ring (and eventually to the clipboard). I eventually came up with a simple solution. This make org-mode tables very comfortable for longer Web formularies with complex data, like bank accounts. Best, /PA On Wed, 8 May 2024 at 20:14, Drew Adams <drew.adams@oracle.com> wrote: > >> At the risk of veering off-topic (I mainly care about adding > >> 'bounds-of-thing-at-point-provider-alist' and > >> 'forward-thing-provider-alist'), would adding a new optional STRICT > >> argument to 'thing-at-point' and friends be an ok resolution for > >> everyone? This argument would enable Drew's proposed behavior. That way, > >> users get all the nice behavior by default just like today, and > >> programmers who require strict correctness in their code also have an > >> option. > > > > 1+ if STRICT means thing starting at point > > and without it we may need to look for the > > beginning of the thing (lazy point setting).... > > Why "starting" at point? STRICT shouldn't mean > checking whether a THING starts at point. > > The "strict" behavior that's needed is checking > _whether_ there is a THING at point, that is, > checking whether the char at point (which really > means just after point) is on/inside a THING. > > That's for `bounds-of-thing-at-point' etc. For > thing-at-point etc., IF there's a THING at point > then that THING is the non-nil value returned. > > In terms of implementation, I suggest you take > a look at the code in thingatpt+.el. Look at > functions `tap-bounds-of-thing-at-point' and > `tap-thing-at-point'. Just remove the `tap-' > prefix for code that DTRT. The file Commentary > explains things in detail. > > My code adds an optional arg SYNTAX-TABLE, but > you need not bother with that. Elisp now has > `with-syntax-table', which can be used as a > workaround if there's no such argument. > > In the end, `bounds-of-thing-at-point' and > `thing-at-point' should return nil if there's > no THING at the char at point. > > For "lax" behavior that corresponds to the > current vanilla behavior, if the strict > behavior would return nil then they return > what the strict behavior would return at > (1- point). > > But really such a lax behavior is pretty lame. > What's needed, for trying to grab a THING near > point is not just checking backward one char > but checking backward, forward, up, and down > N chars, where N determines what "near" means. > > thingatpt+.el provides two user options, > `tap-near-point-x-distance' and > `tap-near-point-y-distance', the max number of > chars from point to check for a THING (for X: > left and right, for Y: up and down). Setting > the Y value to zero constrains search to the > same line as point. > -- Fragen sind nicht da, um beantwortet zu werden, Fragen sind da um gestellt zu werden Georg Kreisler Headaches with a Juju log: unit-basic-16: 09:17:36 WARNING juju.worker.uniter.operation we should run a leader-deposed hook here, but we can't yet [-- Attachment #2: Type: text/html, Size: 4197 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-08 18:14 ` Drew Adams 2024-05-09 5:46 ` Pedro Andres Aranda Gutierrez @ 2024-05-15 8:06 ` Jean Louis 2024-05-15 20:56 ` Drew Adams 1 sibling, 1 reply; 9+ messages in thread From: Jean Louis @ 2024-05-15 8:06 UTC (permalink / raw) To: Drew Adams; +Cc: Pedro Andres Aranda Gutierrez, emacs-devel * Drew Adams <drew.adams@oracle.com> [2024-05-08 21:17]: > >> At the risk of veering off-topic (I mainly care about adding > >> 'bounds-of-thing-at-point-provider-alist' and > >> 'forward-thing-provider-alist'), would adding a new optional STRICT > >> argument to 'thing-at-point' and friends be an ok resolution for > >> everyone? This argument would enable Drew's proposed behavior. That way, > >> users get all the nice behavior by default just like today, and > >> programmers who require strict correctness in their code also have an > >> option. > > > > 1+ if STRICT means thing starting at point > > and without it we may need to look for the > > beginning of the thing (lazy point setting).... I have few functions defining things at point such as ISO date, or things within various parenthesis. I was of understanding that it is the regular expression that decides what is on the point or not. Even space before what someone considers object could mean that user is at the point. Thus there is no need to constrain users defining what is thing at point by some switch like STRICT which would basically break what user intended to do. Just leave it globally available to users to decide how thing at point is defined. -- Jean Take action in Free Software Foundation campaigns: https://www.fsf.org/campaigns ✡️🛡️ Proudly standing with Israel, a nation rooted in history and culture. Let's condemn hatred and promote understanding. In support of Richard M. Stallman https://stallmansupport.org/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-15 8:06 ` Jean Louis @ 2024-05-15 20:56 ` Drew Adams 2024-05-15 21:25 ` Jean Louis 0 siblings, 1 reply; 9+ messages in thread From: Drew Adams @ 2024-05-15 20:56 UTC (permalink / raw) To: Jean Louis; +Cc: Pedro Andres Aranda Gutierrez, emacs-devel > * Drew Adams <drew.adams@oracle.com> [2024-05-08 21:17]: > > >> At the risk of veering off-topic (I mainly care about adding > > >> 'bounds-of-thing-at-point-provider-alist' and > > >> 'forward-thing-provider-alist'), would adding a new optional STRICT > > >> argument to 'thing-at-point' and friends be an ok resolution for > > >> everyone? This argument would enable Drew's proposed behavior. That > way, > > >> users get all the nice behavior by default just like today, and > > >> programmers who require strict correctness in their code also have an > > >> option. > > > > > > 1+ if STRICT means thing starting at point > > > and without it we may need to look for the > > > beginning of the thing (lazy point setting).... Please don't (presumably accidentally) make it look like you're responding to something I wrote, but you actually quote two others - nothing at all from me. Thx. > I was of understanding that it is the regular > expression that decides what is on the point or not. What regular expression? Are you thinking of a regular expression that's checked to see if some particular kind of thing is at point, e.g. a regexp like the value of a var such as `thing-at-point-newsgroup-regexp'? Such a regexp doesn't determine/define what "at point" means. The question here is where you test for a thing at point, i.e., what "at point" means. It's not about how you test that. If your test function actually checks something at a char that's not at point, that's its prerogative. It's about the generic behavior of Thing At Point. The char at point (which actually means the char just after point) is well-defined. That's what the generic functions `bounds-of-thing-at-point' and `thing-at-point' use as starting point. Then then move forward and backward a THING etc. to see if there's such a THING at that starting point. That movement is customizable (with properties `forward-op', `beginning-op', `end-op', `bounds-of-thing-at-point', `thing-at-point', and in other ways). But the starting position is `(point)'; that's where the check is done. The bug is in `bounds-of-thing-at-point'. It should be faithful to its name, and check only at (point), not at EITHER point OR (1- (point)). Even if the fix were to always check only at (1- (point)), instead of (point), it would be a legitimate fix for the problem caused by the ambiguity: there's _no way to determine whether_ there's a thing at a given position. The bugged behavior defeats the successive use of functions that do something with _the next thing_ at point when that _next thing butts up against the previous thing_. IOW, it technically doesn't matter which single position is the "origin" for checking: (point) or (1- (point)) or whatever else. The bug is that _no single_ position is used. However, since "at point" does mean something in Emacs English, the proper fix is to always check starting from (point), not (1- (point)) or any other position. Either always-(point) or always-(1- (point)) would be a fix. But always-(point) is the better/proper fix, just because it accords with Emacs terminology. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-15 20:56 ` Drew Adams @ 2024-05-15 21:25 ` Jean Louis 2024-05-15 21:56 ` Drew Adams 0 siblings, 1 reply; 9+ messages in thread From: Jean Louis @ 2024-05-15 21:25 UTC (permalink / raw) To: Drew Adams; +Cc: Pedro Andres Aranda Gutierrez, emacs-devel * Drew Adams <drew.adams@oracle.com> [2024-05-15 23:57]: > > I was of understanding that it is the regular > > expression that decides what is on the point or not. > > What regular expression? > > Are you thinking of a regular expression that's > checked to see if some particular kind of thing > is at point, e.g. a regexp like the value of a > var such as `thing-at-point-newsgroup-regexp'? > > Such a regexp doesn't determine/define what "at > point" means. Maybe I have not call it right. I may explain it on the example: ;;; Thing at point 'iso-date We assume that rcd-rx-iso-date ➜ "[[:digit:]][[:digit:]][[:digit:]][[:digit:]]-\\(?:0[[:digit:]]\\|1[0-2]\\)-\\(?:[0-2][[:digit:]]\\|3[01]\\)" (defun rcd-tap-iso-date-start () "Move point to the beginning of the ISO date." (when (thing-at-point-looking-at rcd-rx-iso-date) (goto-char (match-beginning 0)))) (defun rcd-tap-iso-date-end () "Move point to the end of the ISO date." (when (thing-at-point-looking-at rcd-rx-iso-date) (goto-char (match-end 0)))) (put 'iso-date 'beginning-op 'rcd-tap-iso-date-start) (put 'iso-date 'end-op 'rcd-tap-iso-date-end) The definition of the thing at point is the one deciding what is the thing. And programmer is the one deciding where the thing actually begins. While user may have different view point where the thing at point begins, the actual definition is somewhere in the code and can be found and understood. I could define that my ISO date is surrounded by spaces " 2024-05-16 " and user may assume that it is not: "2024-05-16", so the definition is in the code and left for programmers to decide how to define it. What if I wish that thing at point is anywhere on the line, regardless where is the cursor? While I did not try it out, I think that is quite feasible to do. Then why try to make general STRICT, to try to affect specific random or indefinite definitions of code, instead, complaint or suggestion, patch on specific code can be made to make it stricter, or new code may be written to make that what other user wish to achieve. > That movement is customizable (with properties > `forward-op', `beginning-op', `end-op', > `bounds-of-thing-at-point', `thing-at-point', > and in other ways). But the starting position > is `(point)'; that's where the check is done. Yes, I agree on everything you said with the exception not mentioned, and that is the consideration of programmer. Programmer may say that ISO date below is at point, when cursor or point is at X, and that ISO date shall be extracted or memorized, noted, etc. by follow up functions. It is matter of consideration. ..X.......................... 2024-05-16 ISO date need not be like below X to say that it is "at point" as it is matter of consideration. I see your words that you said the same what I am saying here. 2X24-05-16 > The bug is in `bounds-of-thing-at-point'. It > should be faithful to its name, and check only > at (point), not at EITHER point OR (1- (point)). 1. The thing at point is defined by programmer. 2. What are bounds of thing at point is thus result of definition of programmer. 3. It implies that thing at point may be looking like having spaces or being forward or backwards from cursor, and still be considered at point, as that is defined by programmer. I do not consider that a bug. 2024-05-16 A cursors before the above ISO date is not captured, and after it, is not captured as thing at point. I have tried it with M-: (thing-at-point 'iso-date), though I believe that other thing at point may be defined so to be sensed close to cursor, and that freedom is fine. What is the "bound" is defined by programmer not by function "bounds-of-thing-at-point". > Even if the fix were to always check only at > (1- (point)), instead of (point), it would be > a legitimate fix for the problem caused by the > ambiguity: there's _no way to determine whether_ > there's a thing at a given position. I see that as freedom, and you see as ambiguity. Without looking into the code and understanding it, isn't it all of the code at some point ambiguity? > IOW, it technically doesn't matter which single > position is the "origin" for checking: (point) > or (1- (point)) or whatever else. The bug is > that _no single_ position is used. I see that function `thing-at-point-looking-at' is using function `point'. You are here expert, you see some bug there, and I see freedom of considerations. > However, since "at point" does mean something > in Emacs English, the proper fix is to always > check starting from (point), not (1- (point)) > or any other position. I did not experience that with the ISO date, so I do not see it is bug. Maybe it is, as I forgot what I did to make it work. But I do understand that other things at point are invoked when cursor is before it. And that is freedom to define it, it does not need to be strictly on the point. My searchable text area maybe a line, not so? And I wish to capture or record that date on the line, while I did not make it so, it may be useful. Point could be anywhere far from the ISO date, and invoking function could capture the date. I find that useful. > Either always-(point) or always-(1- (point)) > would be a fix. But always-(point) is the > better/proper fix, just because it accords > with Emacs terminology. Sure, but where is that (1- (point))? I did not find it. I was using this function below, and I do not see that. I tried searching for "(1- )" and did not find that reference, but found something for the URL https://www.EXAMPLE.comX - if I place cursor on X it will detect the URL. But that is because URL thing was defined that way to be detected. Maybe you should complain on that specific function to be changed, instead of trying to change general code in present and in future with STRICT. The function below has DISTANCE argument for those settings. (defun thing-at-point-looking-at (regexp &optional distance) "Return non-nil if point is in or just after a match for REGEXP. Set the match data from the earliest such match ending at or after point. Optional argument DISTANCE limits search for REGEXP forward and back from point." (let* ((old (point)) (beg (if distance (max (point-min) (- old distance)) (point-min))) (end (if distance (min (point-max) (+ old distance)))) prev match) (save-excursion (goto-char beg) (while (and (setq prev (point) match (re-search-forward regexp end t)) (< (match-end 0) old)) (goto-char (match-beginning 0)) ;; Avoid inflooping when `regexp' matches the empty string. (unless (< prev (point)) (forward-char)))) (and match (<= (match-beginning 0) old (match-end 0))))) What if I think of visible buffer as thing at point? Why not? Then I wish to capture specific text inside, and no matter where is cursor, it would work. I find that handy, useful. -- Jean Take action in Free Software Foundation campaigns: https://www.fsf.org/campaigns ✡️🛡️ Proudly standing with Israel, a nation rooted in history and culture. Let's condemn hatred and promote understanding. In support of Richard M. Stallman https://stallmansupport.org/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-15 21:25 ` Jean Louis @ 2024-05-15 21:56 ` Drew Adams 0 siblings, 0 replies; 9+ messages in thread From: Drew Adams @ 2024-05-15 21:56 UTC (permalink / raw) To: Jean Louis; +Cc: Pedro Andres Aranda Gutierrez, emacs-devel > > The bug is in `bounds-of-thing-at-point'. It > > should be faithful to its name, and check only > > at (point), not at EITHER point OR (1- (point)). > > Even if the fix were to always check only at > > (1- (point)), instead of (point), it would be > > a legitimate fix for the problem caused by the > > ambiguity: there's _no way to determine whether_ > > there's a thing at a given position. > > IOW, it technically doesn't matter which single > > position is the "origin" for checking: (point) > > or (1- (point)) or whatever else. The bug is > > that _no single_ position is used. You're missing the point, even though you quoted some of it (above). You didn't quote this part: The bugged behavior defeats the _successive_ use of functions that do something with the next thing at point when that next thing butts up against the previous thing. Define a `go-to-next-thing' command that you can repeat, to go to successive things of given kind, for any kind of thing. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Add support for 'thing-at-point' to get URL at point @ 2023-11-06 19:45 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, 1 reply; 9+ 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] 9+ 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 20:11 ` Ihor Radchenko 2023-11-06 20:53 ` Jim Porter 0 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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-04-12 12:41 ` Ihor Radchenko 0 siblings, 1 reply; 9+ 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] 9+ 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-04-12 12:41 ` Ihor Radchenko 2024-04-12 22:30 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Ihor Radchenko @ 2024-04-12 12:41 UTC (permalink / raw) To: Jim Porter; +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). It has been a while since the last message in this thread. Jim, may I know if you had a chance to work on the patch? -- 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] 9+ messages in thread
* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-12 12:41 ` Ihor Radchenko @ 2024-04-12 22:30 ` Jim Porter 2024-04-29 4:26 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-04-12 22:30 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-devel, emacs-orgmode On 4/12/2024 5:41 AM, Ihor Radchenko wrote: > Jim Porter <jporterbugs@gmail.com> writes: > >> 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). > > It has been a while since the last message in this thread. > Jim, may I know if you had a chance to work on the patch? Sorry about that. I'm currently extremely swamped with real life, but most of that should be wrapped up by the end of the month, at which point I'll be able to devote some time to Emacs again. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-12 22:30 ` Jim Porter @ 2024-04-29 4:26 ` Jim Porter 2024-04-29 18:14 ` Ihor Radchenko 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-04-29 4:26 UTC (permalink / raw) To: Ihor Radchenko, eliz; +Cc: emacs-devel, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1231 bytes --] On 4/12/2024 3:30 PM, Jim Porter wrote: > On 4/12/2024 5:41 AM, Ihor Radchenko wrote: >> Jim Porter <jporterbugs@gmail.com> writes: >> >>> 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). >> >> It has been a while since the last message in this thread. >> Jim, may I know if you had a chance to work on the patch? > > Sorry about that. I'm currently extremely swamped with real life, but > most of that should be wrapped up by the end of the month, at which > point I'll be able to devote some time to Emacs again. Ihor, Eli: What do you think of the attached patch? I added variables to let modes define custom providers for 'bounds-of-thing-at-point' and 'forward-thing'. (Notably, I avoided adding vars for the 'beginning-of-thing' and 'end-of-thing' functions, since those just call 'bounds-of-thing-at-point' anyway.) If this looks like a reasonable way to go, I'll continue work on this patch by adding entries to 'bounds-of-thing-at-point-provider-alist' and 'forward-thing-provider-alist' in the appropriate places (i.e. wherever we already add to 'thing-at-point-provider-alist', like in EWW). [-- Attachment #2: 0001-Allow-defining-custom-providers-for-more-thingatpt-f.patch --] [-- Type: text/plain, Size: 6486 bytes --] From a0ed62aa42fa47043511ba814cf5ce8419e9d03f Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Sun, 28 Apr 2024 21:19:53 -0700 Subject: [PATCH] Allow defining custom providers for more "thingatpt" functions * lisp/thingatpt.el (bounds-of-thing-at-point-provider-alist) (forward-thing-provider-alist): New variables... (forward-thing, bounds-of-thing-at-point): ... use them. * test/lisp/thingatpt-tests.el (thing-at-point-providers) (forward-thing-providers, bounds-of-thing-at-point-providers): New tests. * etc/NEWS: Announce this change. --- etc/NEWS | 18 +++++++++++++----- lisp/thingatpt.el | 35 ++++++++++++++++++++++++++++++----- test/lisp/thingatpt-tests.el | 31 +++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 7efb4110bcd..2480f0d096d 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1591,19 +1591,27 @@ of the currently existing keyboards macros using the new mode duplicating them, deleting them, and editing their counters, formats, and keys. -** Miscellaneous +** thingatpt.el --- -*** Webjump now assumes URIs are HTTPS instead of HTTP. -For links in 'webjump-sites' without an explicit URI scheme, it was -previously assumed that they should be prefixed with "http://". Such -URIs are now prefixed with "https://" instead. +*** New variables for providing custom thingatpt implementations. +The new variables 'bounds-of-thing-at-point-provider-alist' and +'forward-thing-provider-alist' now allow defining custom implementations +of 'bounds-of-thing-at-point' and 'forward-thing', respectively. --- *** 'bug-reference-mode' now supports 'thing-at-point'. Now, calling '(thing-at-point 'url)' when point is on a bug reference will return the URL for that bug. +** Miscellaneous + +--- +*** Webjump now assumes URIs are HTTPS instead of HTTP. +For links in 'webjump-sites' without an explicit URI scheme, it was +previously assumed that they should be prefixed with "http://". Such +URIs are now prefixed with "https://" instead. + +++ *** New user option 'rcirc-log-time-format' This allows for rcirc logs to use a custom timestamp format, than the diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el index 7896ad984df..d5f71e3c6a8 100644 --- a/lisp/thingatpt.el +++ b/lisp/thingatpt.el @@ -75,6 +75,22 @@ thing-at-point-provider-alist `existing-filename', `url', `email', `uuid', `word', `sentence', `whitespace', `line', `face' and `page'.") +(defvar bounds-of-thing-at-point-provider-alist nil + "Alist of providers to return the bounds of a \"thing\" at point. +This variable can be set globally, or appended to buffer-locally by +modes, to provide functions that will return the bounds of a \"thing\" +at point. The first provider for the \"thing\" that returns a non-nil +value wins. You can use this in much the same way as +`thing-at-point-provider-alist' (which see).") + +(defvar forward-thing-provider-alist nil + "Alist of providers for moving forward to the end of a \"thing\". +This variable can be set globally, or appended to buffer-locally by +modes, to provide functions that will move forward to the end of a +\"thing\" at point. The first provider for the \"thing\" that returns a +non-nil value wins. You can use this in much the same way as +`thing-at-point-provider-alist' (which see).") + ;; Basic movement ;;;###autoload @@ -84,11 +100,16 @@ forward-thing Possibilities include `symbol', `list', `sexp', `defun', `number', `filename', `url', `email', `uuid', `word', `sentence', `whitespace', `line', and `page'." - (let ((forward-op (or (get thing 'forward-op) - (intern-soft (format "forward-%s" thing))))) - (if (functionp forward-op) - (funcall forward-op (or n 1)) - (error "Can't determine how to move over a %s" thing)))) + (setq n (or n 1)) + (or (seq-some (lambda (elt) + (and (eq (car elt) thing) + (funcall (cdr elt) n))) + forward-thing-provider-alist) + (let ((forward-op (or (get thing 'forward-op) + (intern-soft (format "forward-%s" thing))))) + (if (functionp forward-op) + (funcall forward-op n) + (error "Can't determine how to move over a %s" thing))))) ;; General routines @@ -106,6 +127,10 @@ bounds-of-thing-at-point Return a cons cell (START . END) giving the start and end positions of the thing found." (cond + ((seq-some (lambda (elt) + (and (eq (car elt) thing) + (funcall (cdr elt)))) + bounds-of-thing-at-point-provider-alist)) ((get thing 'bounds-of-thing-at-point) (funcall (get thing 'bounds-of-thing-at-point))) ;; If the buffer is totally empty, give up. diff --git a/test/lisp/thingatpt-tests.el b/test/lisp/thingatpt-tests.el index e50738f1122..4aacd776176 100644 --- a/test/lisp/thingatpt-tests.el +++ b/test/lisp/thingatpt-tests.el @@ -258,4 +258,35 @@ test-numbers-hex-c (should (equal (test--number "0xf00" 2) 3840)) (should (equal (test--number "0xf00" 3) 3840))) +(ert-deftest thing-at-point-providers () + (with-temp-buffer + (setq-local thing-at-point-provider-alist + `((url . ,(lambda () "test")))) + (insert "hello") + (should (equal (thing-at-point 'url) "test")) + (should (equal (thing-at-point 'word) "hello")))) + +(ert-deftest forward-thing-providers () + (with-temp-buffer + (setq-local forward-thing-provider-alist + `((url . ,(lambda (n) (goto-char 4))))) + (insert "hello there") + (goto-char (point-min)) + (should (eq (save-excursion (forward-thing 'url) (point)) 4)) + (should (eq (save-excursion (forward-thing 'word) (point)) 6)))) + +(ert-deftest bounds-of-thing-at-point-providers () + (with-temp-buffer + (setq-local bounds-of-thing-at-point-provider-alist + `((url . ,(lambda () '(2 . 3))))) + (insert "hello") + ;; Look for a "URL", using our provider above. + (should (equal (bounds-of-thing-at-point 'url) '(2 . 3))) + (should (eq (save-excursion (beginning-of-thing 'url)) 2)) + (should (eq (save-excursion (end-of-thing 'url)) 3)) + ;; Look for a word, which should *not* use our provider above. + (should (equal (bounds-of-thing-at-point 'word) '(1 . 6))) + (should (eq (save-excursion (beginning-of-thing 'word)) 1)) + (should (eq (save-excursion (end-of-thing 'word)) 6)))) + ;;; thingatpt-tests.el ends here -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-29 4:26 ` Jim Porter @ 2024-04-29 18:14 ` Ihor Radchenko 2024-04-30 4:42 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Ihor Radchenko @ 2024-04-29 18:14 UTC (permalink / raw) To: Jim Porter; +Cc: eliz, emacs-devel, emacs-orgmode Jim Porter <jporterbugs@gmail.com> writes: > Ihor, Eli: What do you think of the attached patch? I added variables to > let modes define custom providers for 'bounds-of-thing-at-point' and > 'forward-thing'. (Notably, I avoided adding vars for the > 'beginning-of-thing' and 'end-of-thing' functions, since those just call > 'bounds-of-thing-at-point' anyway.) > > If this looks like a reasonable way to go, I'll continue work on this > patch by adding entries to 'bounds-of-thing-at-point-provider-alist' and > 'forward-thing-provider-alist' in the appropriate places (i.e. wherever > we already add to 'thing-at-point-provider-alist', like in EWW). Thanks! I have a small comment on the docstring of `forward-thing-provider-alist' - it refers to `thing-at-point-provider-alist', but the provides here are called with an argument N, unlike the providers in `thing-at-point-provider-alist'. -- 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] 9+ messages in thread
* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-29 18:14 ` Ihor Radchenko @ 2024-04-30 4:42 ` Jim Porter 2024-04-30 11:39 ` Ihor Radchenko 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-04-30 4:42 UTC (permalink / raw) To: Ihor Radchenko; +Cc: eliz, emacs-devel, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 633 bytes --] On 4/29/2024 11:14 AM, Ihor Radchenko wrote: > Thanks! > I have a small comment on the docstring of > `forward-thing-provider-alist' - it refers to > `thing-at-point-provider-alist', but the provides here are called with > an argument N, unlike the providers in `thing-at-point-provider-alist'. Fixed. I've also added some helper functions for 'forward-thing' and 'bounds-of-thing-at-point' when the "thing" is defined by a text property, and then used those helper functions for EWW and bug-reference-mode. I've lightly tested this (and added a few automated regression tests), but there could be some bugs lurking in here... [-- Attachment #2: 0001-Allow-defining-custom-providers-for-more-thingatpt-f.patch --] [-- Type: text/plain, Size: 12593 bytes --] From ad8db930907cd760142fd6f035d97ce93ce8d850 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Sun, 28 Apr 2024 21:19:53 -0700 Subject: [PATCH] Allow defining custom providers for more "thingatpt" functions * lisp/thingatpt.el (forward-thing-provider-alist) (bounds-of-thing-at-point-provider-alist): New variables... (forward-thing, bounds-of-thing-at-point): ... use them. (text-property-search-forward, text-property-search-backward) (prop-match-beginning, prop-match-end): Declare. (forward-thing-for-text-property) (bounds-of-thing-at-point-for-text-property): New functions. * lisp/net/eww.el (eww--bounds-of-url-at-point, eww--forward-url): New functions... (eww-mode): ... use them. * lisp/progmodes/bug-reference.el (bug-reference--bounds-of-url-at-point, bug-reference--forward-url): New functions... (bug-reference--init): ... use them. * test/lisp/thingatpt-tests.el (thing-at-point-providers) (forward-thing-providers, bounds-of-thing-at-point-providers): New tests. * etc/NEWS: Announce this change. --- etc/NEWS | 21 +++++++--- lisp/net/eww.el | 14 +++++++ lisp/progmodes/bug-reference.el | 22 +++++++++- lisp/thingatpt.el | 71 ++++++++++++++++++++++++++++++--- test/lisp/thingatpt-tests.el | 36 +++++++++++++++++ 5 files changed, 153 insertions(+), 11 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 7efb4110bcd..394f75884c1 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1591,19 +1591,30 @@ of the currently existing keyboards macros using the new mode duplicating them, deleting them, and editing their counters, formats, and keys. -** Miscellaneous +** thingatpt.el --- -*** Webjump now assumes URIs are HTTPS instead of HTTP. -For links in 'webjump-sites' without an explicit URI scheme, it was -previously assumed that they should be prefixed with "http://". Such -URIs are now prefixed with "https://" instead. +*** New variables and functions for providing custom thingatpt implementations. +The new variables 'bounds-of-thing-at-point-provider-alist' and +'forward-thing-provider-alist' now allow defining custom implementations +of 'bounds-of-thing-at-point' and 'forward-thing', respectively. In +addition, "things" defined by a text property can use the new functions +'bounds-of-thing-at-point-for-text-property' and +'forward-thing-for-text-property' to help implement these providers. --- *** 'bug-reference-mode' now supports 'thing-at-point'. Now, calling '(thing-at-point 'url)' when point is on a bug reference will return the URL for that bug. +** Miscellaneous + +--- +*** Webjump now assumes URIs are HTTPS instead of HTTP. +For links in 'webjump-sites' without an explicit URI scheme, it was +previously assumed that they should be prefixed with "http://". Such +URIs are now prefixed with "https://" instead. + +++ *** New user option 'rcirc-log-time-format' This allows for rcirc logs to use a custom timestamp format, than the diff --git a/lisp/net/eww.el b/lisp/net/eww.el index 39ea964d47a..adabd8d8d8b 100644 --- a/lisp/net/eww.el +++ b/lisp/net/eww.el @@ -1321,6 +1321,12 @@ eww-mode (setq-local thing-at-point-provider-alist (append thing-at-point-provider-alist '((url . eww--url-at-point)))) + (setq-local bounds-of-thing-at-point-provider-alist + (append bounds-of-thing-at-point-provider-alist + '((url . eww--bounds-of-url-at-point)))) + (setq-local forward-thing-provider-alist + (append forward-thing-provider-alist + '((url . eww--forward-url)))) (setq-local bookmark-make-record-function #'eww-bookmark-make-record) (buffer-disable-undo) (setq-local shr-url-transformer #'eww--transform-url) @@ -1351,6 +1357,14 @@ eww--url-at-point "`thing-at-point' provider function." (get-text-property (point) 'shr-url)) +(defun eww--bounds-of-url-at-point () + "`bounds-of-thing-at-point' provider function." + (bounds-of-thing-at-point-for-text-property 'shr-url)) + +(defun eww--forward-url (n) + "`forward-thing' provider function." + (forward-thing-for-text-property 'shr-url n)) + ;;;###autoload (defun eww-browse-url (url &optional new-window) "Ask the EWW browser to load URL. diff --git a/lisp/progmodes/bug-reference.el b/lisp/progmodes/bug-reference.el index 977a3d72cb7..bfc22fb10d2 100644 --- a/lisp/progmodes/bug-reference.el +++ b/lisp/progmodes/bug-reference.el @@ -660,17 +660,37 @@ bug-reference--url-at-point "`thing-at-point' provider function." (get-char-property (point) 'bug-reference-url)) +(defun bug-reference--bounds-of-url-at-point () + "`bounds-of-thing-at-point' provider function." + (bounds-of-thing-at-point-for-text-property 'bug-reference-url)) + +(defun bug-reference--forward-url (n) + "`forward-thing' provider function." + (forward-thing-for-text-property 'bug-reference-url n)) + (defun bug-reference--init (enable) (if enable (progn (jit-lock-register #'bug-reference-fontify) (setq-local thing-at-point-provider-alist (append thing-at-point-provider-alist - '((url . bug-reference--url-at-point))))) + '((url . bug-reference--url-at-point)))) + (setq-local bounds-of-thing-at-point-provider-alist + (append bounds-of-thing-at-point-provider-alist + '((url . bug-reference--bounds-of-url-at-point)))) + (setq-local forward-thing-provider-alist + (append forward-thing-provider-alist + '((url . bug-reference--forward-url))))) (jit-lock-unregister #'bug-reference-fontify) (setq thing-at-point-provider-alist (delete '((url . bug-reference--url-at-point)) thing-at-point-provider-alist)) + (setq bounds-of-thing-at-point-provider-alist + (delete '((url . bug-reference--bounds-of-url-at-point)) + bounds-of-thing-at-point-provider-alist)) + (setq forward-thing-provider-alist + (delete '((url . bug-reference--forward-url)) + forward-thing-provider-alist)) (save-restriction (widen) (bug-reference-unfontify (point-min) (point-max))))) diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el index 7896ad984df..dad71a4ca94 100644 --- a/lisp/thingatpt.el +++ b/lisp/thingatpt.el @@ -75,6 +75,27 @@ thing-at-point-provider-alist `existing-filename', `url', `email', `uuid', `word', `sentence', `whitespace', `line', `face' and `page'.") +(defvar forward-thing-provider-alist nil + "Alist of providers for moving forward to the end of a \"thing\". +This variable can be set globally, or appended to buffer-locally by +modes, to provide functions that will move forward to the end of a +\"thing\" at point. Each function should take a single argument N, the +number of \"things\" to move forward past. The first provider for the +\"thing\" that returns a non-nil value wins. + +You can use this variable in much the same way as +`thing-at-point-provider-alist' (which see).") + +(defvar bounds-of-thing-at-point-provider-alist nil + "Alist of providers to return the bounds of a \"thing\" at point. +This variable can be set globally, or appended to buffer-locally by +modes, to provide functions that will return the bounds of a \"thing\" +at point. The first provider for the \"thing\" that returns a non-nil +value wins. + +You can use this variable in much the same way as +`thing-at-point-provider-alist' (which see).") + ;; Basic movement ;;;###autoload @@ -84,11 +105,16 @@ forward-thing Possibilities include `symbol', `list', `sexp', `defun', `number', `filename', `url', `email', `uuid', `word', `sentence', `whitespace', `line', and `page'." - (let ((forward-op (or (get thing 'forward-op) - (intern-soft (format "forward-%s" thing))))) - (if (functionp forward-op) - (funcall forward-op (or n 1)) - (error "Can't determine how to move over a %s" thing)))) + (setq n (or n 1)) + (or (seq-some (lambda (elt) + (and (eq (car elt) thing) + (funcall (cdr elt) n))) + forward-thing-provider-alist) + (let ((forward-op (or (get thing 'forward-op) + (intern-soft (format "forward-%s" thing))))) + (if (functionp forward-op) + (funcall forward-op n) + (error "Can't determine how to move over a %s" thing))))) ;; General routines @@ -106,6 +132,10 @@ bounds-of-thing-at-point Return a cons cell (START . END) giving the start and end positions of the thing found." (cond + ((seq-some (lambda (elt) + (and (eq (car elt) thing) + (funcall (cdr elt)))) + bounds-of-thing-at-point-provider-alist)) ((get thing 'bounds-of-thing-at-point) (funcall (get thing 'bounds-of-thing-at-point))) ;; If the buffer is totally empty, give up. @@ -775,4 +805,35 @@ list-at-point (goto-char (or (nth 8 ppss) (point))) (form-at-point 'list 'listp)))) +(autoload 'text-property-search-forward "text-property-search") +(autoload 'text-property-search-backward "text-property-search") +(autoload 'prop-match-beginning "text-property-search") +(autoload 'prop-match-end "text-property-search") + +(defun forward-thing-for-text-property (property n) + "Move forward to the end of the Nth next \"thing\". +Each \"thing\" is a region of text with the specified text PROPERTY set." + (let ((search-func (if (> n 0) #'text-property-search-forward + #'text-property-search-backward)) + (pos-func (if (> n 0) #'prop-match-end #'prop-match-beginning)) + (limit (if (> n 0) (point-max) (point-min)))) + (catch 'done + (dotimes (_ (abs n)) + (if-let ((match (funcall search-func property))) + (goto-char (funcall pos-func match)) + (goto-char limit) + (throw 'done t)))) + ;; Return non-nil. + t)) + +(defun bounds-of-thing-at-point-for-text-property (property) + "Determine the start and end buffer locations for the \"thing\" at point. +The \"thing\" is a region of text with the specified text PROPERTY set." + (when (get-text-property (point) property) + (cons (or (previous-single-property-change + (min (1+ (point)) (point-max)) property) + (point-min)) + (or (next-single-property-change (point) property) + (point-max))))) + ;;; thingatpt.el ends here diff --git a/test/lisp/thingatpt-tests.el b/test/lisp/thingatpt-tests.el index e50738f1122..26e20f58be7 100644 --- a/test/lisp/thingatpt-tests.el +++ b/test/lisp/thingatpt-tests.el @@ -258,4 +258,40 @@ test-numbers-hex-c (should (equal (test--number "0xf00" 2) 3840)) (should (equal (test--number "0xf00" 3) 3840))) +(ert-deftest thing-at-point-providers () + (with-temp-buffer + (setq-local thing-at-point-provider-alist + `((url . ,(lambda () (get-text-property (point) 'my-url))))) + (insert (propertize "hello" 'my-url "test")) + (goto-char (point-min)) + (should (equal (thing-at-point 'url) "test")) + (should (equal (thing-at-point 'word) "hello")))) + +(ert-deftest forward-thing-providers () + (with-temp-buffer + (setq-local forward-thing-provider-alist + `((url . ,(lambda (n) + (forward-thing-for-text-property 'my-url n))))) + (insert (propertize "foo" 'my-url "test") "bar") + (goto-char (point-min)) + (should (eq (save-excursion (forward-thing 'url) (point)) 4)) + (should (eq (save-excursion (forward-thing 'word) (point)) 7)))) + +(ert-deftest bounds-of-thing-at-point-providers () + (with-temp-buffer + (setq-local bounds-of-thing-at-point-provider-alist + `((url . ,(lambda () + (bounds-of-thing-at-point-for-text-property + 'my-url))))) + (insert (propertize "foo" 'my-url "test") "bar") + (goto-char (point-min)) + ;; Look for a "URL", using our provider above. + (should (equal (bounds-of-thing-at-point 'url) '(1 . 4))) + (should (eq (save-excursion (beginning-of-thing 'url)) 1)) + (should (eq (save-excursion (end-of-thing 'url)) 4)) + ;; Look for a word, which should *not* use our provider above. + (should (equal (bounds-of-thing-at-point 'word) '(1 . 7))) + (should (eq (save-excursion (beginning-of-thing 'word)) 1)) + (should (eq (save-excursion (end-of-thing 'word)) 7)))) + ;;; thingatpt-tests.el ends here -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-30 4:42 ` Jim Porter @ 2024-04-30 11:39 ` Ihor Radchenko 2024-04-30 18:27 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Ihor Radchenko @ 2024-04-30 11:39 UTC (permalink / raw) To: Jim Porter; +Cc: eliz, emacs-devel, emacs-orgmode Jim Porter <jporterbugs@gmail.com> writes: > + (setq-local bounds-of-thing-at-point-provider-alist > + (append bounds-of-thing-at-point-provider-alist > + '((url . eww--bounds-of-url-at-point)))) > + (setq-local forward-thing-provider-alist > + (append forward-thing-provider-alist > + '((url . eww--forward-url)))) > ... What happens if you have multiple providers for an URL? You add the provider to the end, so it will have the lower priority in this scenario. I guess that you want the opposite - EWW provider to take precedence. Same for other changes. > +(ert-deftest thing-at-point-providers () > ... > +(ert-deftest forward-thing-providers () > ... > +(ert-deftest bounds-of-thing-at-point-providers () > + (with-temp-buffer > + (setq-local bounds-of-thing-at-point-provider-alist > + `((url . ,(lambda () > + (bounds-of-thing-at-point-for-text-property > + 'my-url))))) It would make sense to add tests for "first wins" behaviour. -- 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] 9+ messages in thread
* Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-30 11:39 ` Ihor Radchenko @ 2024-04-30 18:27 ` Jim Porter 2024-04-30 21:10 ` [External] : " Drew Adams 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-04-30 18:27 UTC (permalink / raw) To: Ihor Radchenko; +Cc: eliz, emacs-devel, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 847 bytes --] On 4/30/2024 4:39 AM, Ihor Radchenko wrote: > What happens if you have multiple providers for an URL? > You add the provider to the end, so it will have the lower priority in > this scenario. I guess that you want the opposite - EWW provider to take > precedence. Same for other changes. That's probably reasonable. I was just keeping things the way they were historically here, but we might as well fix this now. > It would make sense to add tests for "first wins" behaviour. Done. I've also fixed a bug in EWW and bug-reference-mode where it would return nil for (thing-at-point 'url) if point was at the *end* of a URL. It's now consistent with how 'thing-at-point' works by default. (If you have two consecutive URLs and point is between them - only possible with the custom provider function, I think - it'll prefer the second one.) [-- Attachment #2: 0001-Allow-defining-custom-providers-for-more-thingatpt-f.patch --] [-- Type: text/plain, Size: 14978 bytes --] From da26f0160c955f15e123e5b28cf8a9f514395e21 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Sun, 28 Apr 2024 21:19:53 -0700 Subject: [PATCH] Allow defining custom providers for more "thingatpt" functions This also fixes an issue in EWW and bug-reference-mode where (thing-at-point 'url) at the end of a URL would return nil. * lisp/thingatpt.el (forward-thing-provider-alist) (bounds-of-thing-at-point-provider-alist): New variables... (forward-thing, bounds-of-thing-at-point): ... use them. (text-property-search-forward, text-property-search-backward) (prop-match-beginning, prop-match-end): Declare. (thing-at-point-for-text-property, forward-thing-for-text-property) (bounds-of-thing-at-point-for-text-property): New functions. * lisp/net/eww.el (eww--url-at-point): Use 'thing-at-point-for-text-property'. (eww--bounds-of-url-at-point, eww--forward-url): New functions... (eww-mode): ... use them. * lisp/progmodes/bug-reference.el (bug-reference--url-at-point): Use 'thing-at-point-for-text-property'. (bug-reference--bounds-of-url-at-point, bug-reference--forward-url): New functions... (bug-reference--init): ... use them. * test/lisp/thingatpt-tests.el (thing-at-point-providers) (forward-thing-providers, bounds-of-thing-at-point-providers): New tests. * etc/NEWS: Announce this change. --- etc/NEWS | 25 ++++++++-- lisp/net/eww.el | 21 +++++++-- lisp/progmodes/bug-reference.el | 26 +++++++++-- lisp/thingatpt.el | 83 +++++++++++++++++++++++++++++++-- test/lisp/thingatpt-tests.el | 59 +++++++++++++++++++++++ 5 files changed, 198 insertions(+), 16 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 7efb4110bcd..061161bb2fd 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1591,19 +1591,34 @@ of the currently existing keyboards macros using the new mode duplicating them, deleting them, and editing their counters, formats, and keys. -** Miscellaneous +** Thingatpt --- -*** Webjump now assumes URIs are HTTPS instead of HTTP. -For links in 'webjump-sites' without an explicit URI scheme, it was -previously assumed that they should be prefixed with "http://". Such -URIs are now prefixed with "https://" instead. +*** New variables for providing custom thingatpt implementations. +The new variables 'bounds-of-thing-at-point-provider-alist' and +'forward-thing-provider-alist' now allow defining custom implementations +of 'bounds-of-thing-at-point' and 'forward-thing', respectively. + +--- +*** New helper functions for text property-based thingatpt providers. +The new helper functions 'thing-at-point-for-text-property', +'bounds-of-thing-at-point-for-text-property', and +'forward-thing-for-text-property' can help to help implement custom +thingatpt providers for "things" that are defined by a text property. --- *** 'bug-reference-mode' now supports 'thing-at-point'. Now, calling '(thing-at-point 'url)' when point is on a bug reference will return the URL for that bug. +** Miscellaneous + +--- +*** Webjump now assumes URIs are HTTPS instead of HTTP. +For links in 'webjump-sites' without an explicit URI scheme, it was +previously assumed that they should be prefixed with "http://". Such +URIs are now prefixed with "https://" instead. + +++ *** New user option 'rcirc-log-time-format' This allows for rcirc logs to use a custom timestamp format, than the diff --git a/lisp/net/eww.el b/lisp/net/eww.el index 39ea964d47a..b3997786d9e 100644 --- a/lisp/net/eww.el +++ b/lisp/net/eww.el @@ -1318,9 +1318,16 @@ eww-mode ;; desktop support (setq-local desktop-save-buffer #'eww-desktop-misc-data) (setq truncate-lines t) + ;; thingatpt support (setq-local thing-at-point-provider-alist - (append thing-at-point-provider-alist - '((url . eww--url-at-point)))) + (cons '(url . eww--url-at-point) + thing-at-point-provider-alist)) + (setq-local forward-thing-provider-alist + (cons '(url . eww--forward-url) + forward-thing-provider-alist)) + (setq-local bounds-of-thing-at-point-provider-alist + (cons '(url . eww--bounds-of-url-at-point) + bounds-of-thing-at-point-provider-alist)) (setq-local bookmark-make-record-function #'eww-bookmark-make-record) (buffer-disable-undo) (setq-local shr-url-transformer #'eww--transform-url) @@ -1349,7 +1356,15 @@ eww--rescale-images (defun eww--url-at-point () "`thing-at-point' provider function." - (get-text-property (point) 'shr-url)) + (thing-at-point-for-text-property 'shr-url)) + +(defun eww--forward-url (n) + "`forward-thing' provider function." + (forward-thing-for-text-property 'shr-url n)) + +(defun eww--bounds-of-url-at-point () + "`bounds-of-thing-at-point' provider function." + (bounds-of-thing-at-point-for-text-property 'shr-url)) ;;;###autoload (defun eww-browse-url (url &optional new-window) diff --git a/lisp/progmodes/bug-reference.el b/lisp/progmodes/bug-reference.el index 977a3d72cb7..be162cf9e11 100644 --- a/lisp/progmodes/bug-reference.el +++ b/lisp/progmodes/bug-reference.el @@ -658,19 +658,39 @@ bug-reference--run-auto-setup (defun bug-reference--url-at-point () "`thing-at-point' provider function." - (get-char-property (point) 'bug-reference-url)) + (thing-at-point-for-text-property 'bug-reference-url)) + +(defun bug-reference--forward-url (n) + "`forward-thing' provider function." + (forward-thing-for-text-property 'bug-reference-url n)) + +(defun bug-reference--bounds-of-url-at-point () + "`bounds-of-thing-at-point' provider function." + (bounds-of-thing-at-point-for-text-property 'bug-reference-url)) (defun bug-reference--init (enable) (if enable (progn (jit-lock-register #'bug-reference-fontify) (setq-local thing-at-point-provider-alist - (append thing-at-point-provider-alist - '((url . bug-reference--url-at-point))))) + (cons '(url . bug-reference--url-at-point) + thing-at-point-provider-alist)) + (setq-local forward-thing-provider-alist + (cons '(url . bug-reference--forward-url) + forward-thing-provider-alist)) + (setq-local bounds-of-thing-at-point-provider-alist + (cons '(url . bug-reference--bounds-of-url-at-point) + bounds-of-thing-at-point-provider-alist))) (jit-lock-unregister #'bug-reference-fontify) (setq thing-at-point-provider-alist (delete '((url . bug-reference--url-at-point)) thing-at-point-provider-alist)) + (setq forward-thing-provider-alist + (delete '((url . bug-reference--forward-url)) + forward-thing-provider-alist)) + (setq bounds-of-thing-at-point-provider-alist + (delete '((url . bug-reference--bounds-of-url-at-point)) + bounds-of-thing-at-point-provider-alist)) (save-restriction (widen) (bug-reference-unfontify (point-min) (point-max))))) diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el index 7896ad984df..825f49cfab7 100644 --- a/lisp/thingatpt.el +++ b/lisp/thingatpt.el @@ -75,6 +75,27 @@ thing-at-point-provider-alist `existing-filename', `url', `email', `uuid', `word', `sentence', `whitespace', `line', `face' and `page'.") +(defvar forward-thing-provider-alist nil + "Alist of providers for moving forward to the end of a \"thing\". +This variable can be set globally, or appended to buffer-locally by +modes, to provide functions that will move forward to the end of a +\"thing\" at point. Each function should take a single argument N, the +number of \"things\" to move forward past. The first provider for the +\"thing\" that returns a non-nil value wins. + +You can use this variable in much the same way as +`thing-at-point-provider-alist' (which see).") + +(defvar bounds-of-thing-at-point-provider-alist nil + "Alist of providers to return the bounds of a \"thing\" at point. +This variable can be set globally, or appended to buffer-locally by +modes, to provide functions that will return the bounds of a \"thing\" +at point. The first provider for the \"thing\" that returns a non-nil +value wins. + +You can use this variable in much the same way as +`thing-at-point-provider-alist' (which see).") + ;; Basic movement ;;;###autoload @@ -84,11 +105,16 @@ forward-thing Possibilities include `symbol', `list', `sexp', `defun', `number', `filename', `url', `email', `uuid', `word', `sentence', `whitespace', `line', and `page'." - (let ((forward-op (or (get thing 'forward-op) - (intern-soft (format "forward-%s" thing))))) - (if (functionp forward-op) - (funcall forward-op (or n 1)) - (error "Can't determine how to move over a %s" thing)))) + (setq n (or n 1)) + (or (seq-some (lambda (elt) + (and (eq (car elt) thing) + (funcall (cdr elt) n))) + forward-thing-provider-alist) + (let ((forward-op (or (get thing 'forward-op) + (intern-soft (format "forward-%s" thing))))) + (if (functionp forward-op) + (funcall forward-op n) + (error "Can't determine how to move over a %s" thing))))) ;; General routines @@ -106,6 +132,10 @@ bounds-of-thing-at-point Return a cons cell (START . END) giving the start and end positions of the thing found." (cond + ((seq-some (lambda (elt) + (and (eq (car elt) thing) + (funcall (cdr elt)))) + bounds-of-thing-at-point-provider-alist)) ((get thing 'bounds-of-thing-at-point) (funcall (get thing 'bounds-of-thing-at-point))) ;; If the buffer is totally empty, give up. @@ -775,4 +805,47 @@ list-at-point (goto-char (or (nth 8 ppss) (point))) (form-at-point 'list 'listp)))) +;; Provider helper functions + +(defun thing-at-point-for-text-property (property) + "Return the \"thing\" at point. +Each \"thing\" is a region of text with the specified text PROPERTY set." + (or (get-text-property (point) property) + (and (> (point) (point-min)) + (get-text-property (1- (point)) property)))) + +(autoload 'text-property-search-forward "text-property-search") +(autoload 'text-property-search-backward "text-property-search") +(autoload 'prop-match-beginning "text-property-search") +(autoload 'prop-match-end "text-property-search") + +(defun forward-thing-for-text-property (property n) + "Move forward to the end of the Nth next \"thing\". +Each \"thing\" is a region of text with the specified text PROPERTY set." + (let ((search-func (if (> n 0) #'text-property-search-forward + #'text-property-search-backward)) + (pos-func (if (> n 0) #'prop-match-end #'prop-match-beginning)) + (limit (if (> n 0) (point-max) (point-min)))) + (catch 'done + (dotimes (_ (abs n)) + (if-let ((match (funcall search-func property))) + (goto-char (funcall pos-func match)) + (goto-char limit) + (throw 'done t)))) + ;; Return non-nil. + t)) + +(defun bounds-of-thing-at-point-for-text-property (property) + "Determine the start and end buffer locations for the \"thing\" at point. +The \"thing\" is a region of text with the specified text PROPERTY set." + (let ((pos (point))) + (when (or (get-text-property pos property) + (and (> pos (point-min)) + (get-text-property (setq pos (1- pos)) property))) + (cons (or (previous-single-property-change + (min (1+ pos) (point-max)) property) + (point-min)) + (or (next-single-property-change pos property) + (point-max)))))) + ;;; thingatpt.el ends here diff --git a/test/lisp/thingatpt-tests.el b/test/lisp/thingatpt-tests.el index e50738f1122..88a4bc8a27d 100644 --- a/test/lisp/thingatpt-tests.el +++ b/test/lisp/thingatpt-tests.el @@ -258,4 +258,63 @@ test-numbers-hex-c (should (equal (test--number "0xf00" 2) 3840)) (should (equal (test--number "0xf00" 3) 3840))) +(ert-deftest thing-at-point-providers () + (with-temp-buffer + (setq-local + thing-at-point-provider-alist + `((url . ,(lambda () (thing-at-point-for-text-property 'foo-url))) + (url . ,(lambda () (thing-at-point-for-text-property 'bar-url))))) + (insert (propertize "hello" 'foo-url "foo.com") "\n" + (propertize "goodbye" 'bar-url "bar.com")) + (goto-char (point-min)) + ;; Get the URL using the first provider. + (should (equal (thing-at-point 'url) "foo.com")) + (should (equal (thing-at-point 'word) "hello")) + (goto-char (point-max)) + ;; Get the URL using the second provider. + (should (equal (thing-at-point 'url) "bar.com")))) + +(ert-deftest forward-thing-providers () + (with-temp-buffer + (setq-local + forward-thing-provider-alist + `((url . ,(lambda (n) (forward-thing-for-text-property 'foo-url n))) + (url . ,(lambda (n) (forward-thing-for-text-property 'bar-url n))))) + (insert (propertize "hello" 'foo-url "foo.com") "there\n" + (propertize "goodbye" 'bar-url "bar.com")) + (goto-char (point-min)) + (save-excursion + (forward-thing 'url) ; Move past the first URL. + (should (= (point) 6)) + (forward-thing 'url) ; Move past the second URL. + (should (= (point) 19))) + (goto-char (point-min)) ; Go back to the beginning... + (forward-thing 'word) ; ... and move past the first word. + (should (= (point) 11)))) + +(ert-deftest bounds-of-thing-at-point-providers () + (with-temp-buffer + (setq-local + bounds-of-thing-at-point-provider-alist + `((url . ,(lambda () + (bounds-of-thing-at-point-for-text-property 'foo-url))) + (url . ,(lambda () + (bounds-of-thing-at-point-for-text-property 'bar-url))))) + (insert (propertize "hello" 'foo-url "foo.com") "there\n" + (propertize "goodbye" 'bar-url "bar.com")) + (goto-char (point-min)) + ;; Look for a URL, using the first provider above. + (should (equal (bounds-of-thing-at-point 'url) '(1 . 6))) + (should (eq (save-excursion (beginning-of-thing 'url)) 1)) + (should (eq (save-excursion (end-of-thing 'url)) 6)) + ;; Look for a word, which should *not* use our provider above. + (should (equal (bounds-of-thing-at-point 'word) '(1 . 11))) + (should (eq (save-excursion (beginning-of-thing 'word)) 1)) + (should (eq (save-excursion (end-of-thing 'word)) 11)) + (goto-char (point-max)) + ;; Look for a URL, using the second provider above. + (should (equal (bounds-of-thing-at-point 'url) '(12 . 19))) + (should (eq (save-excursion (beginning-of-thing 'url)) 12)) + (should (eq (save-excursion (end-of-thing 'url)) 19)))) + ;;; thingatpt-tests.el ends here -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-30 18:27 ` Jim Porter @ 2024-04-30 21:10 ` Drew Adams 2024-05-07 1:08 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Drew Adams @ 2024-04-30 21:10 UTC (permalink / raw) To: Jim Porter, Ihor Radchenko Cc: eliz@gnu.org, emacs-devel@gnu.org, emacs-orgmode@gnu.org > I've also fixed a bug in EWW and bug-reference-mode > where it would return nil for (thing-at-point 'url) > if point was at the *end* of a URL. By "at the end" I assume you really mean just _after_ a URL, i.e., no longer on/at the URL. FWIW, that's actually _superior_ behavior. Unfortunately however, Emacs has chosen the behavior you describe here: > It's now consistent with how 'thing-at-point' > works by default. > (If you have two consecutive URLs and point > is between them...it'll prefer the second one.) Which is better! It's what "at point" means. (Yes, technically point is between the chars.) And with a block cursor the cursor is on the second thing, not the first. And `C-x =' describes the current "cursor position" (aka point), and it describes - wait for it - not the char before point but the char after point, IOW, colloquially the char at point. And `forward-sexp', `forward-word', `forward-thing', etc. advance just _past_ the thing. The cursor is then _not_ on the thing, and unless the thing is immediately followed by another thing, there's _no_ thing at point. Unfortunately, Emacs maintainers decided that thingatpt.el isn't useful for anything except obtaining something to use as a default value for user input. The opinion was that no one ever wants/needs to get nil, telling them that there's no thing at point. Better, they think, to always try to get a thing at point OR at (1- point). This awful Emacs behavior defeats the successive use of functions that do something with the next thing at point, in precisely the case you cited: when the next thing butts up against the previous thing. In particular, these important use cases are defeated by the behavior chosen for Emacs: 1. To find out _whether there is_, in fact, a THING at point. AT POINT - not point OR (point - 1). 2. IF there really is a THING at point, to return it (or its bounds). See bug #9300, " `bounds-of-thing-at-point' does not return nil when just after THING". ___ Library thingatpt+.el fixes this, providing more useful behavior for thing-at-point, and making more use cases possible. It also provides functions for picking up a thing that's _near_ point (where "near" can be specified). That's what Emacs _should_ do for the only use case it even cares about, which is trying to get a thing for use as a default value for input. Getting a thing near point is quite different from getting a thing _at point_. ___ https://www.emacswiki.org/emacs/download/thingatpt%2b.el ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RE: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-04-30 21:10 ` [External] : " Drew Adams @ 2024-05-07 1:08 ` Jim Porter 2024-05-07 1:52 ` Drew Adams 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-05-07 1:08 UTC (permalink / raw) To: Drew Adams, Ihor Radchenko Cc: eliz@gnu.org, emacs-devel@gnu.org, emacs-orgmode@gnu.org On 4/30/2024 2:10 PM, Drew Adams wrote: >> I've also fixed a bug in EWW and bug-reference-mode >> where it would return nil for (thing-at-point 'url) >> if point was at the *end* of a URL. > > By "at the end" I assume you really mean just > _after_ a URL, i.e., no longer on/at the URL. > > FWIW, that's actually _superior_ behavior. > > Unfortunately however, Emacs has chosen the > behavior you describe here: > >> It's now consistent with how 'thing-at-point' >> works by default. > >> (If you have two consecutive URLs and point >> is between them...it'll prefer the second one.) > > Which is better! It's what "at point" means. [snip] > See bug #9300, " `bounds-of-thing-at-point' > does not return nil when just after THING". I agree overall that your proposed behavior is more correct, and it's probably how I'd have implemented 'thing-at-point' if I were doing it from scratch. However, I think an even worse outcome than "thing-at-point looks at point or before-point" is "sometimes thing-at-point just looks at point, and other times it looks at point or before-point" (which is what it does today). I'd even be open to something like a 'thing-at-point-is-strict' defvar that people could let-bind as wanted, but I'm not going to *argue* for that myself. Ultimately though, this patch is really just about providing the necessary defcustoms for org-mode to be able to use 'thing-at-point' (and for Ihor to feel ok about it ;)). Changing 'thing-at-point's behavior should probably be handled separately, especially since there'd be an uphill battle to revisit the decision in bug#9300. ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: RE: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) 2024-05-07 1:08 ` Jim Porter @ 2024-05-07 1:52 ` Drew Adams 0 siblings, 0 replies; 9+ messages in thread From: Drew Adams @ 2024-05-07 1:52 UTC (permalink / raw) To: Jim Porter, Ihor Radchenko Cc: eliz@gnu.org, emacs-devel@gnu.org, emacs-orgmode@gnu.org Thanks for your reply, Jim. > On 4/30/2024 2:10 PM, Drew Adams wrote: > >> I've also fixed a bug in EWW and bug-reference-mode > >> where it would return nil for (thing-at-point 'url) > >> if point was at the *end* of a URL. > > > > By "at the end" I assume you really mean just > > _after_ a URL, i.e., no longer on/at the URL. > > > > FWIW, that's actually _superior_ behavior. > > > > Unfortunately however, Emacs has chosen the > > behavior you describe here: > > > >> It's now consistent with how 'thing-at-point' > >> works by default. > > > >> (If you have two consecutive URLs and point > >> is between them...it'll prefer the second one.) > > > > Which is better! It's what "at point" means. > > > [snip] > > > > See bug #9300, " `bounds-of-thing-at-point' > > does not return nil when just after THING". > > I agree overall that your proposed behavior is more correct, and it's > probably how I'd have implemented 'thing-at-point' if I were doing it > from scratch. However, I think an even worse outcome than > "thing-at-point looks at point or before-point" is "sometimes > thing-at-point just looks at point, and other times it looks at point or > before-point" (which is what it does today). Yes, such inconsistency is arguably worse than consistent bad behavior. Arguably worse - and arguably better. (That's in the nature of inconsistency - some will see a glass half full; others half empty.) > I'd even be open to something like a 'thing-at-point-is-strict' defvar > that people could let-bind as wanted, but I'm not going to *argue* for > that myself. > > Ultimately though, this patch is really just about providing the > necessary defcustoms for org-mode to be able to use 'thing-at-point' > (and for Ihor to feel ok about it ;)). Changing 'thing-at-point's > behavior should probably be handled separately, especially since there'd > be an uphill battle to revisit the decision in bug#9300. I hear you. The behavior should be changed so that, in general, bounds-of-thing-at-point etc. return nil when there is _no thing at point_, including when point is after, including just after, a thing but not on such a thing. There can be commands (and noncommand fns) that return things _near_ point, not only at point. And "near" can be configurable with an argument. In particular, they can do what the vanilla fns currently do: return a thing at OR just before point. But the "-at-point" functions shouldn't do that. They should do what their names say. It's important to have such functions. It's not just about arguing that strictly at-point is better than at-or-just-after-point. The point (sic) is that currently there's _no way to determine whether_ there's a thing at point. That's the real problem - no test for a thing at a given position. That means that a whole slew of possible applications of the feature are impossible to realize. ___ Along with the fix for this bug, there could also be a defvar, or even an option, that fuses the two behaviors (at OR just before), i.e., that gives the current, misguided behavior, to allow existing code or habits compatibility. It's not hard for Emacs to still DTRT. It just takes a decision and admission that the behavior was misguided and unnecessarily limiting (BIG time). ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-05-15 21:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-08 16:09 Re: [External] : Re: Adding custom providers for thingatpt.el (was: [PATCH] Add support for 'thing-at-point' to get URL at point) Pedro Andres Aranda Gutierrez 2024-05-08 18:14 ` Drew Adams 2024-05-09 5:46 ` Pedro Andres Aranda Gutierrez 2024-05-15 8:06 ` Jean Louis 2024-05-15 20:56 ` Drew Adams 2024-05-15 21:25 ` Jean Louis 2024-05-15 21:56 ` Drew Adams -- strict thread matches above, loose matches on Subject: below -- 2023-11-06 19:45 [PATCH] Add support for 'thing-at-point' to get URL at point 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-04-12 12:41 ` Ihor Radchenko 2024-04-12 22:30 ` Jim Porter 2024-04-29 4:26 ` Jim Porter 2024-04-29 18:14 ` Ihor Radchenko 2024-04-30 4:42 ` Jim Porter 2024-04-30 11:39 ` Ihor Radchenko 2024-04-30 18:27 ` Jim Porter 2024-04-30 21:10 ` [External] : " Drew Adams 2024-05-07 1:08 ` Jim Porter 2024-05-07 1:52 ` Drew Adams
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.