bug#9454: 24.0.50; thingatpt.el should be updated to respect field boundaries

bug#9454: 24.0.50; thingatpt.el should be updated to respect field boundaries

[Drew Adams]

I'm no expert on fields, to put it mildly.  But my impression is that
thing-at-point functions should not cross field boundaries.
 
I ran into this using a thing-at-point function (actually, a
thing-near-point function) in the minibuffer.  Now that (forward-line 0)
takes you across the field boundary and into the prompt field, I ended
up with "Text is read-only" errors.
 
(And just why is it an improvement to have two fields here?  What was
wrong with the (simple) situation before?)
 
To fix this I ended up splattering my own thing-at-point code with
`constrain-to-field' calls.  Butt ugly, but unless I'm missing something
(quite possible), unavoidable.
 
Anyway, this is a heads-up that you might want to take a look at
thingatpt.el and DTRT wrt field boudaries.  FWIW, my own code is here:
http://www.emacswiki.org/emacs/download/thingatpt%2b.el.  Among other
things, it contains fixes for Emacs bugs #8667, #8628, and #9300 (fixes
already communicated, but still not applied to vanilla Emacs).
 

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-09-05 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt'
 

bug#9454: 24.0.50; thingatpt.el should be updated to respect field boundaries

[Lars Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> I'm no expert on fields, to put it mildly.  But my impression is that
> thing-at-point functions should not cross field boundaries.
>
> I ran into this using a thing-at-point function (actually, a
> thing-near-point function) in the minibuffer.  Now that (forward-line 0)
> takes you across the field boundary and into the prompt field, I ended
> up with "Text is read-only" errors.

Do you have a test case to reproduce this bug?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9454: 24.0.50; thingatpt.el should be updated to respect field boundaries

[Drew Adams]

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

> Do you have a test case to reproduce this bug?

No.

See the original report.  It points you to my
code that fixes the problem by sprinkling
`constrain-to-field' everywhere.

To give an idea, attached are two functions,
my version and the vanilla version.  Ediff
them.  See the additions of this:

 (constrain-to-field nil orig)

I believe that the vanilla function needs
similar protection.  For you to decide.

That's just one thingatpt.el function.  I
also added `constraint-to-field to other
such functions.

See thingatpt+.el if you want to see the
occurrences, to maybe save some time when
checking where thingatpt.el might need the
same treatment.

I think you get the idea.  Thing-at-point
should respect fields.  Do it or don't do
it; your choice.

[-- Attachment #2: throw-my-thg.el --]
[-- Type: application/octet-stream, Size: 2701 bytes --]

(defun tap-bounds-of-thing-at-point-1 (thing)
  "Helper for `tap-bounds-of-thing-at-point'.
Do everything except handle the optional SYNTAX-TABLE arg."
  (let ((bounds-fn  (or (get thing 'tap-bounds-of-thing-at-point)
                        (get thing 'bounds-of-thing-at-point))))
    (if bounds-fn
        (funcall bounds-fn)
      (let ((orig  (point)))
        (condition-case nil
            (save-excursion
              ;; Try moving forward, then back.
              (funcall (or (get thing 'end-op) ; Move to end.
                           (lambda () (forward-thing thing 1))))
              (constrain-to-field nil orig)
              (funcall (or (get thing 'beginning-op) ; Move to beg.
                           (lambda () (forward-thing thing -1))))
              (constrain-to-field nil orig)
              (let ((beg  (point)))
                (if (<= beg orig)
                    ;; If that brings us all the way back to ORIG,
                    ;; it worked.  But END may not be the real end.
                    ;; So find the real end that corresponds to BEG.
                    ;; FIXME: in which cases can `real-end' differ from `end'?
                    (let ((real-end  (progn (funcall
                                             (or (get thing 'end-op)
                                                 (lambda () (forward-thing thing 1))))
                                            (constrain-to-field nil orig)
                                            (point))))
                      (and (< orig real-end)  (< beg real-end)
                           (cons beg real-end)))
                  (goto-char orig)
                  ;; Try a second time, moving first backward and then forward,
                  ;; so that we can find a thing that ends at ORIG.
                  (funcall (or (get thing 'beginning-op) ; Move to beg.
                               (lambda () (forward-thing thing -1))))
                  (constrain-to-field nil orig)
                  (funcall (or (get thing 'end-op) ; Move to end.
                               (lambda () (forward-thing thing 1))))
                  (constrain-to-field nil orig)
                  (let ((end       (point))
                        (real-beg  (progn (funcall
                                           (or (get thing 'beginning-op)
                                               (lambda () (forward-thing thing -1))))
                                          (constrain-to-field nil orig)
                                          (point))))
                    (and (<= real-beg orig)  (< orig end)  (< real-beg end)
                         (cons real-beg end))))))
          (error nil))))))

[-- Attachment #3: throw-vanilla-thg.el --]
[-- Type: application/octet-stream, Size: 2034 bytes --]

(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',
`filename', `url', `email', `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."
  (if (get thing 'bounds-of-thing-at-point)
      (funcall (get thing 'bounds-of-thing-at-point))
    (let ((orig (point)))
      (ignore-errors
	(save-excursion
	  ;; Try moving forward, then back.
	  (funcall ;; First move to end.
	   (or (get thing 'end-op)
	       (lambda () (forward-thing thing 1))))
	  (funcall ;; Then move to beg.
	   (or (get thing 'beginning-op)
	       (lambda () (forward-thing thing -1))))
	  (let ((beg (point)))
	    (if (<= beg orig)
		;; If that brings us all the way back to ORIG,
		;; it worked.  But END may not be the real end.
		;; So find the real end that corresponds to BEG.
		;; FIXME: in which cases can `real-end' differ from `end'?
		(let ((real-end
		       (progn
			 (funcall
			  (or (get thing 'end-op)
			      (lambda () (forward-thing thing 1))))
			 (point))))
		  (when (and (<= orig real-end) (< beg real-end))
		    (cons beg real-end)))
	      (goto-char orig)
	      ;; Try a second time, moving backward first and then forward,
	      ;; so that we can find a thing that ends at ORIG.
	      (funcall ;; First, move to beg.
	       (or (get thing 'beginning-op)
		   (lambda () (forward-thing thing -1))))
	      (funcall ;; Then move to end.
	       (or (get thing 'end-op)
		   (lambda () (forward-thing thing 1))))
	      (let ((end (point))
		    (real-beg
		     (progn
		       (funcall
			(or (get thing 'beginning-op)
			    (lambda () (forward-thing thing -1))))
		       (point))))
		(if (and (<= real-beg orig) (<= orig end) (< real-beg end))
		    (cons real-beg end))))))))))

bug#9454: 24.0.50; thingatpt.el should be updated to respect field boundaries

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

>> Do you have a test case to reproduce this bug?
>
> No.
>
> See the original report.  It points you to my
> code that fixes the problem by sprinkling
> `constrain-to-field' everywhere.

I understand the problem conceptually, but without a test case to verify
that there's a problem here in practice, it's difficult to say whether
the proposed fix fixes the problem.

This is why we (in the bug reporting process) encourage sending in test
cases, because that makes everything easier.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#9454: 24.0.50; thingatpt.el should be updated to respect field boundaries

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I understand the problem conceptually, but without a test case to verify
> that there's a problem here in practice, it's difficult to say whether
> the proposed fix fixes the problem.

I've now pushed a fix to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no