* bug#34506: 27.0.50: push-button bug with basic text-property button
@ 2019-02-16 22:08 Bob Weiner
2019-02-17 15:24 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Bob Weiner @ 2019-02-16 22:08 UTC (permalink / raw
To: 34506
From the Elisp manual (https://www.gnu.org/software/emacs/manual/html_node/elisp/Manipulating-Buttons.html), we have these accurate explanations:
— Function: button-at pos
Return the button at position pos in the current buffer, or nil. If the button at pos is a text property button, the return value is a marker pointing to pos.
— Function: button-activate button &optional use-mouse-action
Call button's action property (i.e., invoke the function that is the value of that property, passing it the single argument button). If use-mouse-action is non-nil, try to invoke the button's mouse-action property instead of action; if the button has no mouse-action property, use action as normal.
-----
With point on a "Choose" button in a customize-group buffer, point is on
a text-property button and (button-at (point)) returns a marker object
rather than a button whose action is a marker object. Thus, if one
calls (push-button) at that location, it sends this marker object as the
button argument to 'button-activate' which then triggers an error when
it tries to funcall the button's action which is nil in this case.
Shouldn't there be additional logic that checks if the button itself is
a marker and then uses the button as the action in that case?
Related to this: (button-type (button-at (point))) returns nil which seems
to contradict the fact that button-at returns non-nil.
Am I missing things here or does button-activate need additional code?
Thanks,
Bob
-------
In GNU Emacs 27.0.50 (build 13, x86_64-apple-darwin16.7.0, NS appkit-1504.83 Version 10.12.6 (Build 16G1036))
of 2017-12-17 built on bka-iMac.local
Repository revision: 36375d35aa06e84865cce678559ddfa8f79a9775
Windowing system distributor 'Apple', version 10.3.1561
Recent messages:
[Sat 04:46:35 PM]
[Sat 04:46:35 PM]
[Sat 04:46:35 PM] Result: nil
[Sat 04:46:37 PM]
[Sat 04:46:37 PM]
[Sat 04:46:37 PM] Result: nil
[Sat 04:46:38 PM]
[Sat 04:46:38 PM] nil
[Sat 04:47:16 PM]
Back to top level
Configured using:
'configure --with-ns --with-imagemagick --without-pop --with-mailutils
CC=clang 'CFLAGS=-O3 -g''
Configured features:
JPEG NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS LCMS2
Important settings:
value of $LANG: en_US.UTF-8
locale-coding-system: utf-8-unix
Major mode: Custom
Minor modes in effect:
recentf-mode: t
treemacs-follow-mode: t
treemacs-filewatch-mode: t
treemacs-git-mode: deferred
treemacs-fringe-indicator-mode: t
async-bytecomp-package-mode: t
diff-auto-refine-mode: t
desktop-save-mode: t
winner-mode: t
which-key-mode: t
which-function-mode: t
persistent-scratch-autosave-mode: t
global-edit-server-edit-mode: t
delete-selection-mode: t
auto-compile-on-load-mode: t
auto-compile-on-save-mode: t
column-number-indicator-zero-based: t
dynamic-completion-mode: t
eros-mode: t
shell-dirtrack-mode: t
show-paren-mode: t
global-company-mode: t
company-mode: t
ace-window-display-mode: t
major-mode-icons-mode: t
minibuffer-depth-indicate-mode: t
tooltip-mode: t
global-eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
column-number-mode: t
line-number-mode: t
transient-mark-mode: t
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-16 22:08 bug#34506: 27.0.50: push-button bug with basic text-property button Bob Weiner
@ 2019-02-17 15:24 ` Eli Zaretskii
2019-02-17 23:46 ` Robert Weiner
0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-02-17 15:24 UTC (permalink / raw
To: Bob Weiner; +Cc: 34506
> From: Bob Weiner <rsw@gnu.org>
> Date: Sat, 16 Feb 2019 17:08:47 -0500
>
> With point on a "Choose" button in a customize-group buffer, point is on
> a text-property button and (button-at (point)) returns a marker object
> rather than a button whose action is a marker object. Thus, if one
> calls (push-button) at that location, it sends this marker object as the
> button argument to 'button-activate' which then triggers an error when
> it tries to funcall the button's action which is nil in this case.
> Shouldn't there be additional logic that checks if the button itself is
> a marker and then uses the button as the action in that case?
>
> Related to this: (button-type (button-at (point))) returns nil which seems
> to contradict the fact that button-at returns non-nil.
>
> Am I missing things here or does button-activate need additional code?
button-activate and push-button already include that additional code,
but you are trying to invoke them on a kind of "button" that they
don't know how to handle. The problem is that "button" is overloaded
here: buttons created by Customize are not of the kind supported by
functions from button.el, you need to invoke functions described in
widget.info instead. The "text-property buttons" mentioned in the
documentation of button-at etc. are those created by make-text-button
and insert-text-button, not those created by Customize.
The ELisp manual hints on this at the beginning the parent node,
"Buttons". Maybe that's not clear enough; patches to make that more
clear are welcome. Other than that, I don't see a bug here.
Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-17 15:24 ` Eli Zaretskii
@ 2019-02-17 23:46 ` Robert Weiner
2019-02-18 15:47 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Robert Weiner @ 2019-02-17 23:46 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 34506
[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]
On Sun, Feb 17, 2019 at 10:24 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Bob Weiner <rsw@gnu.org>
> > Date: Sat, 16 Feb 2019 17:08:47 -0500
> >
> > With point on a "Choose" button in a customize-group buffer, point is on
> > a text-property button and (button-at (point)) returns a marker object
> > rather than a button whose action is a marker object. Thus, if one
> > calls (push-button) at that location, it sends this marker object as the
> > button argument to 'button-activate' which then triggers an error when
> > it tries to funcall the button's action which is nil in this case.
> > Shouldn't there be additional logic that checks if the button itself is
> > a marker and then uses the button as the action in that case?
> >
> > Related to this: (button-type (button-at (point))) returns nil which
> seems
> > to contradict the fact that button-at returns non-nil.
> >
> > Am I missing things here or does button-activate need additional code?
>
> button-activate and push-button already include that additional code,
> but you are trying to invoke them on a kind of "button" that they
> don't know how to handle. The problem is that "button" is overloaded
> here: buttons created by Customize are not of the kind supported by
> functions from button.el, you need to invoke functions described in
> widget.info instead. The "text-property buttons" mentioned in the
> documentation of button-at etc. are those created by make-text-button
> and insert-text-button, not those created by Customize.
>
I really don't fully understand what you are saying here.
There is a section in widget.info which describes use of the (push-button)
function which is defined in button.el, so although I
understand you are saying there are two different types
of buttons, it seems like they are connected. Maybe
the custom widgets use text-property buttons.
If you could, for each of the two types of buttons,
just show code that finds the button at point and then
activates it, so I can see the difference clearly. That
would be most helpful.
And what about (button-type (button-at (point))) returning
nil when button-at returns non-nil. Both of these functions
operate on push-buttons as the button.el code reflects, right?
If so, then that should be a bug. If not, then it could use
some explanation.
Thanks,
Bob
[-- Attachment #2: Type: text/html, Size: 4677 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-17 23:46 ` Robert Weiner
@ 2019-02-18 15:47 ` Eli Zaretskii
2019-02-18 16:56 ` Robert Weiner
2019-02-18 20:51 ` Basil L. Contovounesios
0 siblings, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2019-02-18 15:47 UTC (permalink / raw
To: rswgnu; +Cc: 34506
> From: Robert Weiner <rsw@gnu.org>
> Date: Sun, 17 Feb 2019 18:46:09 -0500
> Cc: 34506@debbugs.gnu.org
>
> button-activate and push-button already include that additional code,
> but you are trying to invoke them on a kind of "button" that they
> don't know how to handle. The problem is that "button" is overloaded
> here: buttons created by Customize are not of the kind supported by
> functions from button.el, you need to invoke functions described in
> widget.info instead. The "text-property buttons" mentioned in the
> documentation of button-at etc. are those created by make-text-button
> and insert-text-button, not those created by Customize.
>
> I really don't fully understand what you are saying here.
I'm saying that a "button" created by Customize is radically different
from "buttons" that button.el functions can handle. They are
different objects, and it's unfortunate that both kinds are named the
same.
> There is a section in widget.info which describes use of the (push-button)
> function which is defined in button.el
No, push-button described in widget.info is a type of widget, not a
function. Again, the same name used for two very different things.
> Maybe the custom widgets use text-property buttons.
Custom widget button do indeed use text properties, but those
properties are entirely different from the properties used by
text-buttons created by button.el. You can see that yourself by using
the describe-text-properties command with point on each type of
button.
> If you could, for each of the two types of buttons,
> just show code that finds the button at point and then
> activates it, so I can see the difference clearly. That
> would be most helpful.
If you type "C-h c RET" on a button in a Customize buffer, you will
see that RET invokes Custom-newline there -- this is the way to "push"
the button widgets that Custom uses. By contrast, if you do the same
on a button created by button.el, like a hyperlink in the *Help*
buffer, you will see that RET invokes the command push-button there, a
different command. These are the two ways of pushing these two
different kinds of buttons.
> And what about (button-type (button-at (point))) returning
> nil when button-at returns non-nil. Both of these functions
> operate on push-buttons as the button.el code reflects, right?
> If so, then that should be a bug. If not, then it could use
> some explanation.
button-type requires a button as an argument, whereas button-at is
documented to return a marker for text-buttons. So you cannot safely
invoke button-type if the button at point might be of the text-button
type. IOW, the text-button is not an object, it is a place in the
buffer where the button starts.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 15:47 ` Eli Zaretskii
@ 2019-02-18 16:56 ` Robert Weiner
2019-02-18 17:36 ` Eli Zaretskii
2019-02-18 20:52 ` Basil L. Contovounesios
2019-02-18 20:51 ` Basil L. Contovounesios
1 sibling, 2 replies; 16+ messages in thread
From: Robert Weiner @ 2019-02-18 16:56 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 34506
[-- Attachment #1: Type: text/plain, Size: 656 bytes --]
Thanks Eli, that is much clearer. I can work with that explanation.
Your last paragraph indicates that the button API by itself could use some
improvement.
How does one obtain a button to send to button-type if not button-at?
Also, a confusing part of the widget documentation is this:
-------
5.4 The ‘push-button’ Widget
============================
Syntax:
TYPE ::= (push-button [KEYWORD ARGUMENT]... [ VALUE ])
-------
The above syntax description of course looks like a lisp-function call of
push-button.
At the very least, changing this thing's name to widget-button would
prevent such
confusion.
-- Bob
[-- Attachment #2: Type: text/html, Size: 2523 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 16:56 ` Robert Weiner
@ 2019-02-18 17:36 ` Eli Zaretskii
2019-02-18 20:52 ` Basil L. Contovounesios
1 sibling, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2019-02-18 17:36 UTC (permalink / raw
To: rswgnu; +Cc: 34506
> From: Robert Weiner <rsw@gnu.org>
> Date: Mon, 18 Feb 2019 11:56:49 -0500
> Cc: 34506@debbugs.gnu.org
>
> Your last paragraph indicates that the button API by itself could use some improvement.
Possibly.
> How does one obtain a button to send to button-type if not button-at?
Looks like you need to use text-properties-at, and then look at the
result: the type is stored in the 'category' property there.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 15:47 ` Eli Zaretskii
2019-02-18 16:56 ` Robert Weiner
@ 2019-02-18 20:51 ` Basil L. Contovounesios
2019-02-18 22:54 ` Robert Weiner
2019-02-19 3:29 ` Eli Zaretskii
1 sibling, 2 replies; 16+ messages in thread
From: Basil L. Contovounesios @ 2019-02-18 20:51 UTC (permalink / raw
To: Eli Zaretskii; +Cc: rswgnu, 34506
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Robert Weiner <rsw@gnu.org>
>> Date: Sun, 17 Feb 2019 18:46:09 -0500
>> Cc: 34506@debbugs.gnu.org
>>
>> And what about (button-type (button-at (point))) returning
>> nil when button-at returns non-nil. Both of these functions
>> operate on push-buttons as the button.el code reflects, right?
>> If so, then that should be a bug. If not, then it could use
>> some explanation.
>
> button-type requires a button as an argument, whereas button-at is
> documented to return a marker for text-buttons. So you cannot safely
> invoke button-type if the button at point might be of the text-button
> type.
Buffer positions, markers, and overlays all qualify as "buttons", so
button-type works with both text- and overlay-buttons (but not widgets).
So I'm guessing what you meant is "you cannot safely invoke button-type
if the button at point might be a widget rather than a button".
--
Basil
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 16:56 ` Robert Weiner
2019-02-18 17:36 ` Eli Zaretskii
@ 2019-02-18 20:52 ` Basil L. Contovounesios
1 sibling, 0 replies; 16+ messages in thread
From: Basil L. Contovounesios @ 2019-02-18 20:52 UTC (permalink / raw
To: Robert Weiner; +Cc: rswgnu, 34506
Robert Weiner <rsw@gnu.org> writes:
> Thanks Eli, that is much clearer. I can work with that explanation.
>
> Your last paragraph indicates that the button API by itself could use
> some improvement. How does one obtain a button to send to button-type
> if not button-at?
You can use button-at if you want to be agnostic of the button (not
widget) type. If you're only dealing with text (not overlay) buttons,
you can avoid creating markers by passing a buffer position like (point)
directly to button-type.
If you are dealing with widgets, on the other hand, you should use
widget-at and widget-type in place of button-at and button-type,
respectively. The widget and button APIs are mutually incompatible.
--
Basil
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 20:51 ` Basil L. Contovounesios
@ 2019-02-18 22:54 ` Robert Weiner
2019-02-19 3:08 ` Basil L. Contovounesios
2019-02-19 3:29 ` Eli Zaretskii
1 sibling, 1 reply; 16+ messages in thread
From: Robert Weiner @ 2019-02-18 22:54 UTC (permalink / raw
To: Basil L. Contovounesios; +Cc: 34506
[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]
On Mon, Feb 18, 2019 at 3:51 PM Basil L. Contovounesios <contovob@tcd.ie>
wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Robert Weiner <rsw@gnu.org>
> >> Date: Sun, 17 Feb 2019 18:46:09 -0500
> >> Cc: 34506@debbugs.gnu.org
> >>
> >> And what about (button-type (button-at (point))) returning
> >> nil when button-at returns non-nil. Both of these functions
> >> operate on push-buttons as the button.el code reflects, right?
> >> If so, then that should be a bug. If not, then it could use
> >> some explanation.
> >
> > button-type requires a button as an argument, whereas button-at is
> > documented to return a marker for text-buttons. So you cannot safely
> > invoke button-type if the button at point might be of the text-button
> > type.
>
> Buffer positions, markers, and overlays all qualify as "buttons", so
> button-type works with both text- and overlay-buttons (but not widgets).
>
But as I think I noted in my first message, my recollection
is that button-type returned nil when given a marker value
returned from button-at. Since widgets use text-properties,
button-at on a widget can return a non-nil value, so to say that
widgets and buttons are unrelated ignores the programming API.
Maybe the solution is to add a more opaque programming abstraction
atop each type so that they don't expose their underlying implementations
and cause programming errors.
>
> So I'm guessing what you meant is "you cannot safely invoke button-type
> if the button at point might be a widget rather than a button".
>
Yes, again noting that widget documentation specifically mentions
push-buttons
(push-button is the function used to activate buttons). This negates the
idea
that the two constructs are wholly independent of each other.
Regards,
Bob
[-- Attachment #2: Type: text/html, Size: 3817 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 22:54 ` Robert Weiner
@ 2019-02-19 3:08 ` Basil L. Contovounesios
0 siblings, 0 replies; 16+ messages in thread
From: Basil L. Contovounesios @ 2019-02-19 3:08 UTC (permalink / raw
To: Robert Weiner; +Cc: rswgnu, 34506
Robert Weiner <rsw@gnu.org> writes:
> On Mon, Feb 18, 2019 at 3:51 PM Basil L. Contovounesios <contovob@tcd.ie> wrote:
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Robert Weiner <rsw@gnu.org>
> >> Date: Sun, 17 Feb 2019 18:46:09 -0500
> >> Cc: 34506@debbugs.gnu.org
> >>
> >> And what about (button-type (button-at (point))) returning
> >> nil when button-at returns non-nil. Both of these functions
> >> operate on push-buttons as the button.el code reflects, right?
> >> If so, then that should be a bug. If not, then it could use
> >> some explanation.
> >
> > button-type requires a button as an argument, whereas button-at is
> > documented to return a marker for text-buttons. So you cannot safely
> > invoke button-type if the button at point might be of the text-button
> > type.
>
> Buffer positions, markers, and overlays all qualify as "buttons", so
> button-type works with both text- and overlay-buttons (but not widgets).
>
> But as I think I noted in my first message, my recollection
> is that button-type returned nil when given a marker value
> returned from button-at.
I think what's confusing you is that button-at returns a marker even
when there is no button at point. If either of the following two
expressions evaluates to nil, then there is no button at point:
(button-type (button-at (point)))
(button-type (point))
If there is something that *looks* like a button at point, yet these
expressions evaluate to nil, then you're probably looking at a widget
instead.
> Since widgets use text-properties,
AFAICT Customize widgets use overlays, not text properties. I concluded
this by comparing the results of (text-properties-at (point)) and
(overlays-at (point)) with point at a Customize button.
> button-at on a widget can return a non-nil value, so to say that
> widgets and buttons are unrelated ignores the programming API.
Again, that button-at returns a marker for a widget is a coincidence,
not part of either library's API. Last time I read/skimmed the relevant
manuals I was not given the impression that these libraries were
related, but suggestions for clarification of their text is always
welcome.
> Maybe the solution is to add a more opaque programming abstraction
> atop each type so that they don't expose their underlying
> implementations and cause programming errors.
Can you please elaborate? I don't see how either library's
implementation is exposed beyond what is documented in its respective
manual.
> So I'm guessing what you meant is "you cannot safely invoke button-type
> if the button at point might be a widget rather than a button".
>
> Yes, again noting that widget documentation specifically mentions push-buttons
> (push-button is the function used to activate buttons).
push-button is indeed a button.el function used to activate buttons, but
this has nothing to do with the push-button widget type, which is
documented under '(widget) Introduction' and '(widget) push-button'.
> This negates the idea that the two constructs are wholly independent
> of each other.
I hope I have managed to convince you otherwise.
--
Basil
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-18 20:51 ` Basil L. Contovounesios
2019-02-18 22:54 ` Robert Weiner
@ 2019-02-19 3:29 ` Eli Zaretskii
2019-02-19 15:26 ` Basil L. Contovounesios
1 sibling, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-02-19 3:29 UTC (permalink / raw
To: Basil L. Contovounesios; +Cc: rswgnu, 34506
> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: <rswgnu@gmail.com>, <34506@debbugs.gnu.org>
> Date: Mon, 18 Feb 2019 20:51:12 +0000
>
> So I'm guessing what you meant is "you cannot safely invoke button-type
> if the button at point might be a widget rather than a button".
No, I'm trying to explain why button-type returns nil in the case of
text-buttons.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-19 3:29 ` Eli Zaretskii
@ 2019-02-19 15:26 ` Basil L. Contovounesios
2019-02-20 5:22 ` Robert Weiner
0 siblings, 1 reply; 16+ messages in thread
From: Basil L. Contovounesios @ 2019-02-19 15:26 UTC (permalink / raw
To: Eli Zaretskii; +Cc: rswgnu, 34506
Eli Zaretskii <eliz@gnu.org> writes:
>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: <rswgnu@gmail.com>, <34506@debbugs.gnu.org>
>> Date: Mon, 18 Feb 2019 20:51:12 +0000
>>
>> So I'm guessing what you meant is "you cannot safely invoke button-type
>> if the button at point might be a widget rather than a button".
>
> No, I'm trying to explain why button-type returns nil in the case of
> text-buttons.
I'm sorry if I've misunderstood something obvious, but button-type
should never return nil for any kind of button.el button, regardless of
whether it's a text or overlay button.
As stated in '(elisp) Button Types', "every button has a button type".
For example, the following should return 'button:
(with-temp-buffer
(let* ((tstart (prog1 (point) (insert-text-button "text")))
(ostart (prog1 (point) (insert-button "overlay")))
(tbutton (button-at tstart))
(obutton (button-at ostart))
(ttype (button-type tbutton))
(otype (button-type obutton)))
(cl-assert (and tbutton obutton ttype otype))
(cl-assert (eq ttype otype))
(cl-assert (eq ttype (button-type tstart)))
(cl-assert (null (button-type ostart)))
ttype))
[ By the way, would something like this be welcome as a start for
test/lisp/button-tests.el? ]
The only cases (within reason) in which button-type may return nil are:
0. button-type is mistakenly given a buffer position POS instead of an
overlay when the button at POS is an overlay (not text) button, as
shown in the example above. Using (button-at POS) avoids this
problem.
1. button-type is given the value of a widget's (not button's) 'button
overlay property. This is not that far-fetched since both libraries
rely on (get-char-property POS 'button), but I don't see why anyone
would mix widgets and buttons in the same buffer and run the risk of
such gotchas. I believe this contributed in part to Robert's
confusion.
Am I missing something?
--
Basil
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-19 15:26 ` Basil L. Contovounesios
@ 2019-02-20 5:22 ` Robert Weiner
2019-02-25 2:40 ` Basil L. Contovounesios
0 siblings, 1 reply; 16+ messages in thread
From: Robert Weiner @ 2019-02-20 5:22 UTC (permalink / raw
To: Basil L. Contovounesios; +Cc: 34506
> On Feb 19, 2019, at 10:26 AM, Basil L. Contovounesios <contovob@tcd.ie> wrote:
>
> I don't see why anyone
> would mix widgets and buttons in the same buffer and run the risk of
> such gotchas. I believe this contributed in part to Robert's
> confusion.
If buttons and widgets are wholly incompatible and separate then button-at should return nil when on a widget even if both buttons and widgets use the same underlying mechanism. This is what abstractions are for. The implementation should be hidden.
The abstraction I really want is if there is a clickable item under point, activate it in the most appropriate way, e.g. if selected with a mouse event, use it’s mouse action, etc. Maybe an activate-at function that handles different types of objects with a priority order if there are multiple matches.
Bob
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-20 5:22 ` Robert Weiner
@ 2019-02-25 2:40 ` Basil L. Contovounesios
2019-03-02 12:34 ` Eli Zaretskii
0 siblings, 1 reply; 16+ messages in thread
From: Basil L. Contovounesios @ 2019-02-25 2:40 UTC (permalink / raw
To: Robert Weiner; +Cc: 34506
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
Robert Weiner <rswgnu@gmail.com> writes:
>> On Feb 19, 2019, at 10:26 AM, Basil L. Contovounesios <contovob@tcd.ie> wrote:
>>
>> I don't see why anyone
>> would mix widgets and buttons in the same buffer and run the risk of
>> such gotchas. I believe this contributed in part to Robert's
>> confusion.
>
> If buttons and widgets are wholly incompatible and separate then button-at
> should return nil when on a widget even if both buttons and widgets use the same
> underlying mechanism.
How's the following?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: button.diff --]
[-- Type: text/x-diff, Size: 1348 bytes --]
diff --git a/lisp/button.el b/lisp/button.el
index c46f3d9a52..921e84dfa6 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -382,10 +382,12 @@ button-at
If the button at POS is a text property button, the return value
is a marker pointing to POS."
(let ((button (get-char-property pos 'button)))
- (if (or (overlayp button) (null button))
- button
- ;; Must be a text-property button; return a marker pointing to it.
- (copy-marker pos t))))
+ (and button (get-char-property pos 'category)
+ (if (overlayp button)
+ button
+ ;; Must be a text-property button;
+ ;; return a marker pointing to it.
+ (copy-marker pos t)))))
(defun next-button (pos &optional count-current)
"Return the next button after position POS in the current buffer.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 52c0b5b74d..b9f98cdc4c 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1163,8 +1163,9 @@ widget-field-list
(defun widget-at (&optional pos)
"The button or field at POS (default, point)."
- (or (get-char-property (or pos (point)) 'button)
- (widget-field-at pos)))
+ (let ((widget (or (get-char-property (or pos (point)) 'button)
+ (widget-field-at pos))))
+ (and (widgetp widget) widget)))
;;;###autoload
(defun widget-setup ()
[-- Attachment #3: Type: text/plain, Size: 11 bytes --]
--
Basil
^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-02-25 2:40 ` Basil L. Contovounesios
@ 2019-03-02 12:34 ` Eli Zaretskii
2019-04-07 3:14 ` Basil L. Contovounesios
0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2019-03-02 12:34 UTC (permalink / raw
To: Basil L. Contovounesios; +Cc: rswgnu, 34506
> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Eli Zaretskii <eliz@gnu.org>, 34506@debbugs.gnu.org
> Date: Mon, 25 Feb 2019 02:40:26 +0000
>
> > If buttons and widgets are wholly incompatible and separate then button-at
> > should return nil when on a widget even if both buttons and widgets use the same
> > underlying mechanism.
>
> How's the following?
LGTM, but can we please have a simple test for this, so that this
never again regresses?
Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#34506: 27.0.50: push-button bug with basic text-property button
2019-03-02 12:34 ` Eli Zaretskii
@ 2019-04-07 3:14 ` Basil L. Contovounesios
0 siblings, 0 replies; 16+ messages in thread
From: Basil L. Contovounesios @ 2019-04-07 3:14 UTC (permalink / raw
To: Eli Zaretskii; +Cc: rswgnu, 34506-done
[-- Attachment #1: Type: text/plain, Size: 35 bytes --]
tags 34506 fixed
close 34506
quit
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Distinguish-buttons-from-widgets-bug-34506.patch --]
[-- Type: text/x-diff, Size: 5477 bytes --]
From 08235af38c92e95d8ec9d268916d8910ea50ab2d Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sun, 7 Apr 2019 03:36:47 +0100
Subject: [PATCH] Distinguish buttons from widgets (bug#34506)
* lisp/button.el (button-at):
* lisp/wid-edit.el (widget-at): Avoid returning a false positive
when looking for a button and finding a widget, or vice versa.
* test/lisp/button-tests.el:
* test/lisp/wid-edit-tests.el: New files.
---
lisp/button.el | 10 ++++++----
lisp/wid-edit.el | 5 +++--
test/lisp/button-tests.el | 40 +++++++++++++++++++++++++++++++++++++
test/lisp/wid-edit-tests.el | 39 ++++++++++++++++++++++++++++++++++++
4 files changed, 88 insertions(+), 6 deletions(-)
create mode 100644 test/lisp/button-tests.el
create mode 100644 test/lisp/wid-edit-tests.el
diff --git a/lisp/button.el b/lisp/button.el
index c46f3d9a52..921e84dfa6 100644
--- a/lisp/button.el
+++ b/lisp/button.el
@@ -382,10 +382,12 @@ button-at
If the button at POS is a text property button, the return value
is a marker pointing to POS."
(let ((button (get-char-property pos 'button)))
- (if (or (overlayp button) (null button))
- button
- ;; Must be a text-property button; return a marker pointing to it.
- (copy-marker pos t))))
+ (and button (get-char-property pos 'category)
+ (if (overlayp button)
+ button
+ ;; Must be a text-property button;
+ ;; return a marker pointing to it.
+ (copy-marker pos t)))))
(defun next-button (pos &optional count-current)
"Return the next button after position POS in the current buffer.
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 52c0b5b74d..b9f98cdc4c 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -1163,8 +1163,9 @@ widget-field-list
(defun widget-at (&optional pos)
"The button or field at POS (default, point)."
- (or (get-char-property (or pos (point)) 'button)
- (widget-field-at pos)))
+ (let ((widget (or (get-char-property (or pos (point)) 'button)
+ (widget-field-at pos))))
+ (and (widgetp widget) widget)))
;;;###autoload
(defun widget-setup ()
diff --git a/test/lisp/button-tests.el b/test/lisp/button-tests.el
new file mode 100644
index 0000000000..d54a992ab8
--- /dev/null
+++ b/test/lisp/button-tests.el
@@ -0,0 +1,40 @@
+;;; button-tests.el --- tests for button.el -*- lexical-binding: t -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest button-at ()
+ "Test `button-at' behavior."
+ (with-temp-buffer
+ (should-not (button-at (point)))
+ (let ((button (insert-text-button "text button"))
+ (marker (button-at (1- (point)))))
+ (should (markerp marker))
+ (should (= (button-end button) (button-end marker) (point))))
+ (let ((button (insert-button "overlay button"))
+ (overlay (button-at (1- (point)))))
+ (should (overlayp overlay))
+ (should (eq button overlay)))
+ ;; Buttons and widgets are incompatible (bug#34506).
+ (widget-create 'link "link widget")
+ (should-not (button-at (1- (point))))))
+
+;;; button-tests.el ends here
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
new file mode 100644
index 0000000000..a4350e715e
--- /dev/null
+++ b/test/lisp/wid-edit-tests.el
@@ -0,0 +1,39 @@
+;;; wid-edit-tests.el --- tests for wid-edit.el -*- lexical-binding: t -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'wid-edit)
+
+(ert-deftest widget-at ()
+ "Test `widget-at' behavior."
+ (with-temp-buffer
+ (should-not (widget-at))
+ (let ((marco (widget-create 'link "link widget"))
+ (polo (widget-at (1- (point)))))
+ (should (widgetp polo))
+ (should (eq marco polo)))
+ ;; Buttons and widgets are incompatible (bug#34506).
+ (insert-text-button "text button")
+ (should-not (widget-at (1- (point))))
+ (insert-button "overlay button")
+ (should-not (widget-at (1- (point))))))
+
+;;; wid-edit-tests.el ends here
--
2.20.1
[-- Attachment #3: Type: text/plain, Size: 659 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: Eli Zaretskii <eliz@gnu.org>, 34506@debbugs.gnu.org
>> Date: Mon, 25 Feb 2019 02:40:26 +0000
>>
>> > If buttons and widgets are wholly incompatible and separate then button-at
>> > should return nil when on a widget even if both buttons and widgets use the same
>> > underlying mechanism.
>>
>> How's the following?
>
> LGTM, but can we please have a simple test for this, so that this
> never again regresses?
Of course. I've pushed the attached patch to master, and am thus
closing this bug report. Let me know if I missed anything.
Thanks,
--
Basil
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-04-07 3:14 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-16 22:08 bug#34506: 27.0.50: push-button bug with basic text-property button Bob Weiner
2019-02-17 15:24 ` Eli Zaretskii
2019-02-17 23:46 ` Robert Weiner
2019-02-18 15:47 ` Eli Zaretskii
2019-02-18 16:56 ` Robert Weiner
2019-02-18 17:36 ` Eli Zaretskii
2019-02-18 20:52 ` Basil L. Contovounesios
2019-02-18 20:51 ` Basil L. Contovounesios
2019-02-18 22:54 ` Robert Weiner
2019-02-19 3:08 ` Basil L. Contovounesios
2019-02-19 3:29 ` Eli Zaretskii
2019-02-19 15:26 ` Basil L. Contovounesios
2019-02-20 5:22 ` Robert Weiner
2019-02-25 2:40 ` Basil L. Contovounesios
2019-03-02 12:34 ` Eli Zaretskii
2019-04-07 3:14 ` Basil L. Contovounesios
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.