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