bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

emacs -Q
 
In buffer scratch, type this text just after the initial comment:
 
(set-buffer-modified-p nil)
 
;;---------------
 
;;;;;;; fffff
 
;;;;;;; ggggg
;;;;;;; hhhhh
 

Put point just after the list `(set-buffer-modified-p nil)'.
M-: (thing-at-point-bounds-of-list-at-point)
 
The result is (218 . 282), which contains all of tthe text _after_ the
list, up to eob.
 
Put point at eob and do it again:
M-: (thing-at-point-bounds-of-list-at-point)
 
The result is (282 . 282).
 
The result in both cases should be nil - there is *NO* list at point.
 
 
 
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-04-25 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/imagesupport/include'
 

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

Besides which, we might as well get rid of
`thing-at-point-bounds-of-list-at-point' and the `(put 'list
'bounds-of-thing-at-point 'thing-at-point-bounds-of-list-at-point)'.

There is no generally no reason to define such `thing-at-point-bounds-of...'
functions.  The regular `bounds-of-thing-at-point' should work, and it does.  

Try M-: (bound-of-thing-at-point 'list) instead of M-:
(thing-at-point-bounds-of-list-at-point).  It returns (191 . 218) at the first
location (which corresponds to the list before point, which I suppose is OK -
it's certainly a lot better than the current behavior).  And it returns nil at
eob, which is correct.


(Obviously I forgot to mention doing M-x load-library thingatpt.el for the
recipe from emacs -Q.)

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Stefan Monnier]

> Put point just after the list `(set-buffer-modified-p nil)'.
> M-: (thing-at-point-bounds-of-list-at-point)

Nobody should care about thing-at-point-bounds-of-list-at-point because
it's an internal function.


        Stefan

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

> > Put point just after the list `(set-buffer-modified-p nil)'.
> > M-: (thing-at-point-bounds-of-list-at-point)
> 
> Nobody should care about 
> thing-at-point-bounds-of-list-at-point because
> it's an internal function.

An internal function that is called by other functions, which _do_ affect what
users see.

See my followup message to the original report.  The point is that
`bounds-of-thing-at-point' does not work well for type `list'.  And the reason
is that we have `thing-at-point-bounds-of-list-at-point' - it just messes things
up.  Get rid of it and you get nothing but improvement.

Just try it.  Do this and the results for (bounds-of-thing-at-point 'list) are
much better:

(put 'list 'bounds-of-thing-at-point nil)

As I said,

> There is no generally no reason to define such
> `thing-at-point-bounds-of...' functions.  The regular
> `bounds-of-thing-at-point' should work, and it does.  

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Juanma Barranquero]

On Fri, May 13, 2011 at 22:06, Drew Adams <drew.adams@oracle.com> wrote:

> An internal function that is called by other functions, which _do_ affect what
> users see.

Isn't that the very definition of "an internal function"?

    Juanma

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

> > An internal function that is called by other functions, 
> > which _do_ affect what users see.
> 
> Isn't that the very definition of "an internal function"?

Ah, the cleverness.

The point is to fix the user-facing behavior, and the cause of the problem is in
this function.

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Chong Yidong]

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

> Besides which, we might as well get rid of
> `thing-at-point-bounds-of-list-at-point' and the `(put 'list
> 'bounds-of-thing-at-point 'thing-at-point-bounds-of-list-at-point)'.
>
> There is no generally no reason to define such
> `thing-at-point-bounds-of...'  functions.  The regular
> `bounds-of-thing-at-point' should work, and it does.
>
> Try M-: (bound-of-thing-at-point 'list) instead of M-:
>
> (thing-at-point-bounds-of-list-at-point).  It returns (191 . 218) at
> the first location (which corresponds to the list before point, which
> I suppose is OK - it's certainly a lot better than the current
> behavior).  And it returns nil at eob, which is correct.

This is incorrect; thing-at-point-bounds-of-list-at-point is a helper
function that is called by `bounds-of-thing-at-point'.  If removed,
(bounds-of-thing-at-point 'list) no longer works.

I've marked thing-at-point-bounds-of-list-at-point explicitly as an
internal function, and will close this bug.

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

> This is incorrect; thing-at-point-bounds-of-list-at-point is a helper
> function that is called by `bounds-of-thing-at-point'.  If removed,
> (bounds-of-thing-at-point 'list) no longer works.

You're dreaming.  Did you actually try the bug recipe?  Did you then try it
again without `t-a-p-b-o-l-a-p' and its associated `put'?  Which one DTRT?

> I've marked thing-at-point-bounds-of-list-at-point explicitly as an
> internal function,

Irrelevant.  That has nothing to do with this bug.

> and will close this bug.

Wrong.  `thing-at-point-bounds-of-list-at-point' does NOT DTRT - at all.  And
that _causes_ (bounds-of-thing-at-point 'list) to not DTRT.  That is what this
bug report is about: (bounds-of-thing-at-point 'list).

But ordinary (bounds-of-thing-at-point 'list) WILL work, if you just get rid of
`thing-at-point-bounds-of-list-at-point' and its associated `put':

(put 'list 'bounds-of-thing-at-point
     'thing-at-point-bounds-of-list-at-point)

IOW, let `bounds-of-thing-at-point' do its normal thing for `list'.

Please read the bug thread.  Do I really need to repeat it?

Follow the recipe, at least - it's quite clear.  Regardless of whether you like
the fix I suggested, you should at least be able to confirm that the bug exists.

To make things clearer for you, substitute (bounds-of-thing-at-point 'list) for
(thing-at-point-bounds-of-list-at-point) in the recipe.  The former just uses
the latter, so they are the same thing, but apparently you've gotten confused
because I mentioned t-o-p-b-o-l-a-p.

You can see for yourself that (bounds-of-thing-at-point 'list) returns erroneous
results instead of nil when point is not on a list.  Try it.

And no, there is no need for such a "helper" function in the general case and,
as it turns out, in the case of `list' as well.  The idea is that
`bounds-of-thing-at-point' should just work for most THINGs.  And it DOES work
for `list' AFAICT.

More importantly, the helper function `thing-at-point-bounds-of-list-at-point'
does NOT work.  (bounds-of-thing-at-point 'list) should not return non-nil if
point is not on a list.  And `bounds-of-thing-at-point' should never, ever
return an empty thing (equal bounds), as it does here at eol per the recipe.

I just repeated the recipe in the latest Windows build of Emacs 24 - same
problem.

I get the impression that you have not really read this bug report at all.  Your
responses so far have been:

1. "Nobody should care about t-a-p-b-o-l-a-p because it's an internal function."

I do not care about t-a-p-b-o-l-a-p.  I care about the behavior of
`bounds-of-thing-at-point'.  The former is breaking the latter.

2. Just repeating that t-a-p-b-o-l-a-p is an internal function.

This was after I underlined the fact that it is the _behavior_ of
`bounds-of-thing-at-point' that is erroneous, _because of_ t-a-p-b-o-l-a-p.

3. Assignment to the wish list!

Even though I already provided the fix.

4. And now, what you write above, and closing the bug.  Sheesh.


You (pl.) apparently just don't get it.

a. The _behavior_ of `bounds-of-thing-at-point' is erroneous for THING = `list'.
It returns non-nil when point is not on a list in at least two cases, and it
returns an empty thing (empty bounds), which it should _never_ do.

b. It is this _behavior_ that needs to be fixed, however you might like to do
it.  It does not matter whether you choose to use an internal function such as
t-a-p-b-o-l-a-p or not, provided you do _fix the behavior_.

c. The simplest way to fix the behavior, IMHO - but you need not listen to it if
you prefer some other way - is to just get rid of the erroneous
`t-a-p-b-o-l-a-p' (and its associated `put') and let `bound-of-thing-at-point'
do its normal thing.  _That works_ for the cases cited.


Please read the bug report.  Try the recipe.  Fix the problem any way you like,
but please fix it.  This is NOT about internal vs external functions.  This is
about bugged _BEHAVIOR_.

Forget about `t-a-p-b-o-l-a-p'.  This is about the behavior of
`bounds-of-thing-at-point'.  The only reason I mentioned `t-a-p-b-o-l-a-p' is
that that is what screws up the behavior of `bounds-of-thing-at-point'.  Get rid
of the latter and its associated `put' and the former will DTRT for `list'.

Try it.

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Chong Yidong]

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

> You're dreaming.  Did you actually try the bug recipe?  Did you then
> try it again without `t-a-p-b-o-l-a-p' and its associated `put'?
> Which one DTRT?

Removing thing-at-point-bounds-of-list-at-point and its associated put
does not work if point is inside the list, so your suggestion of
removing them is a bogus one.

Maybe there is a bug here, but I'll leave it for some other Emacs
developer who's willing to wade through your abusive posts to find out
what you're going on about.

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

> Maybe there is a bug here,

You can't tell?  Did you try the recipe?  If so, do you have an opinion on
whether the results returned are correct?

It's OK to disagree about whether those results are correct, but I don't hear
you saying anything about that one way or the other.  Can you at least confirm
that you see the same behavior?  Hello?

> but I'll leave it for some other Emacs developer who's willing to wade
> through your abusive posts to find out what you're going on about.

What's to wade through?  The recipe is clear and takes only about 30 seconds to
complete.  30 sec. to reproduce the bug or to determine that you cannot
reproduce it on your system.  You need go no further than the initial bug report
to know what is being reported and how to see whether there is a bug.

There is nothing abusive about repeating that ignored bug recipe, trying to
focus attention on the bug and not on the irrelevant side show of whether
`t-a-p-b-o-l-a-p' is internal or external.

As I made clear, I really do not care _how_ the problem reported gets fixed; my
concern is that it be fixed.

My only complaint was your (pl.) not recognizing the bug, apparently not even
trying the recipe.  That complaint might express understandable frustration on
my part, but it does not represent abuse, so please stop playing the victim.

Not once did anyone even begin to address the bug (recipe) - to say (a) yes, the
behavior is confirmed or no, the behavior cannot be reproduced or (b) the
behavior is confirmed but is as intended (so not a bug).

Instead, the only responses here were to reclassify the report as `wishlist' and
then state that you will close it after marking `t-a-p-b-o-l-a-p' as internal
(no connection with the bug).  The bug is still there.

bug#8628: 24.0.50; `thing-at-point-bounds-of-list-at-point' - no good

[Drew Adams]

FWIW, you can find new definitions of the list-at-point functions, which work
correctly, in my library thingatpt+.el:
http://www.emacswiki.org/emacs/download/thingatpt%2b.el

The definitions are general.
They handle correctly all cases; i.e., where point is:

a. within a list,
b. just after/before or at a list end/beginning, and
c. outside any list.

They return the non-nil list in (a) and (b), and nil in (c).

For the current question, which concerns (bounds-of-thing-at-point 'list), this
is the code I use, in case you want to test:

(put 'list 'bounds-of-thing-at-point 'bounds-of-list-at-point)
(defun bounds-of-list-at-point (&optional up unquotedp)
  "Return (START . END), boundaries of the `list-at-point'.
Return nil if no non-empty list is found.
UP (default: 0) is the number of list levels to go up to start with.
Non-nil UNQUOTEDP means remove the car if it is `quote' or
 `backquote-backquote-symbol'."
  (let ((thing+bds  (list-at-point-with-bounds up unquotedp)))
    (and thing+bds (cdr thing+bds))))

(defun list-at-point-with-bounds (&optional up unquotedp)
  "Return (LIST START . END), boundaries of the `list-at-point'.
Return nil if no non-empty list is found.
UP (default: 0) is the number of list levels to go up to start with.
Non-nil UNQUOTEDP means remove the car if it is `quote' or
 `backquote-backquote-symbol'."
  (list-at/nearest-point-with-bounds
   'sexp-at-point-with-bounds up unquotedp))

(defun list-at/nearest-point-with-bounds (at/near &optional up unquotedp)
  "Helper for `list-at-point-with-bounds' and similar functions.
AT/NEAR is a function called to grab the initial list and its bounds.
UP (default: 0) is the number of list levels to go up to start with.
Non-nil UNQUOTEDP means remove the car if it is `quote' or
 `backquote-backquote-symbol'.
Return (LIST START . END) with START and END of the LIST.
Return nil if no non-empty list is found."
  (save-excursion
    (unless (eq at/near 'sexp-at-point-with-bounds)
      (cond ((looking-at "\\s-*\\s(") (skip-syntax-forward "-"))
            ((looking-at "\\s)\\s-*") (skip-syntax-backward "-"))))
    (let ((sexp+bnds  (funcall at/near)))
      (condition-case nil               ; Handle an `up-list' error.
          (progn
            (when up
              (up-list (- up))
              (setq sexp+bnds  (sexp-at-point-with-bounds)))
            (while (not (consp (car sexp+bnds)))
              (up-list -1)
              (setq sexp+bnds  (sexp-at-point-with-bounds)))
            (when (and unquotedp (consp (car sexp+bnds))
                       (memq (caar sexp+bnds)
                             (list backquote-backquote-symbol 'quote)))
              (cond ((eq 'quote (caar sexp+bnds))
                     (setq sexp+bnds
                           (cons (cadr (car sexp+bnds))
                                 (cons (+ 5 (cadr sexp+bnds))
                                       (cddr sexp+bnds)))))
                    ((eq backquote-backquote-symbol (caar sexp+bnds))
                     (setq sexp+bnds  (cons (cadr (car sexp+bnds))
                                            (cons (+ 1 (cadr sexp+bnds)) 
                                                  (cddr sexp+bnds)))))))
            (while (not (consp (car sexp+bnds)))
              (up-list -1)
              (setq sexp+bnds  (sexp-at-point-with-bounds))))
        (error (setq sexp+bnds  nil)))
      sexp+bnds)))

(defun sexp-at-point-with-bounds (&optional pred syntax-table)
  "Return (SEXP START . END), boundaries of the `sexp-at-point'.
Return nil if no sexp is found.
Optional args are the same as for `form-at-point-with-bounds'."
  (form-at-point-with-bounds 'sexp pred syntax-table))

(defun form-at-point-with-bounds (&optional thing pred syntax-table)
  "Return (FORM START . END), START and END the char positions of FORM.
FORM is the `form-at-point'.  Return nil if no form is found.
Optional arguments:
  THING is the kind of form desired (default: `sexp').
  PRED is a predicate that THING must satisfy to qualify.
  SYNTAX-TABLE is a syntax table to use."
  (condition-case nil              ; E.g. error if tries to read `.'.
      (let* ((thing+bds  (thing-at-point-with-bounds
                          (or thing 'sexp) syntax-table))
             (bounds     (cdr thing+bds))
             (sexp       (and bounds (read-from-whole-string
                                      (car thing+bds)))))
        (and bounds (or (not pred) (funcall pred sexp))
             (cons sexp bounds)))
    (error nil)))

(defun thing-at-point-with-bounds (thing &optional syntax-table)
  "Return (THING START . END) with START and END of THING.
Return nil if no such THING is found.
THING is the `thing-at-point' (which see).
START and END are the car and cdr of the `bounds-of-thing-at-point'.
SYNTAX-TABLE is a syntax table to use."
  (let ((bounds  (bounds-of-thing-at-point thing syntax-table)))
    (and bounds
         (cons (buffer-substring (car bounds) (cdr bounds)) bounds))))

(defun bounds-of-thing-at-point (thing &optional syntax-table)
  "Determine the start and end buffer locations for the THING at point.
Return a consp `(START . END)' giving the START and END positions,
where START /= END.  Return nil if no such THING is found.
THING is an entity for which there is a either a corresponding
`forward-'THING operation, or corresponding `beginning-of-'THING and
`end-of-'THING operations.  THING examples include `word', `sentence',
`defun'.
SYNTAX-TABLE is a syntax table to use.
See the commentary of library `thingatpt.el' for how to define a
symbol as a valid THING."
  (if syntax-table
      (let ((buffer-syntax  (syntax-table)))
        (unwind-protect
             (progn (set-syntax-table syntax-table)
                    (bounds-of-thing-at-point-1 thing))
          (set-syntax-table buffer-syntax)))
    (bounds-of-thing-at-point-1 thing)))

;; This is the original `bounds-of-thing-at-point',
;; but with bug #8667 fixed.
(defun bounds-of-thing-at-point-1 (thing)
  "Helper for `bounds-of-thing-at-point'.
Do all except handle the optional SYNTAX-TABLE arg."
  (if (get thing 'bounds-of-thing-at-point)
      (funcall (get thing 'bounds-of-thing-at-point))
    (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))))
            (funcall (or (get thing 'beginning-op) ; Move to beg.
                         (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))))
                    (and (<= orig real-end) (< beg real-end)
                         (cons beg real-end)))
                (goto-char orig)
                ;; Try a second time, moving first backward 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))))
                (funcall (or (get thing 'end-op) ; Move to end.
                             (lambda () (forward-thing thing 1))))
                (let ((end       (point))
                      (real-beg  (progn
                                   (funcall
                                    (or (get thing 'beginning-op)
                                        (lambda ()
                                         (forward-thing thing -1))))
                                   (point))))
                  (and (<= real-beg orig) (<= orig end) (< real-beg end)
                       (cons real-beg end))))))
        (error nil)))))