GNU bug report logs - #69941
30.0.50; Faulty fontification of radio button widgets

Previous Next

Full log

Message #98 received at 69941 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: 69941 <at> debbugs.gnu.org, Stephen Berman <stephen.berman <at> gmx.net>,
 Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69941: 30.0.50; Faulty fontification of radio button widgets
Date: Fri, 17 Jan 2025 09:12:46 -0500

> I attach a patch that implements something like that, along with a new
> test to check the recursive case.

Great.  A few more comments:

> +  (let* ((parent (widget-get widget :parent))
> +         (parent-from-marker (and parent (widget-get parent :from)))
> +         (parent-to-marker (and parent (widget-get parent :to)))
> +         (continue nil)
> +         (lst nil)
> +         (pos (point)))
> +    (when (and parent-from-marker
> +               (eq pos (marker-position parent-from-marker)))
> +      (set-marker-insertion-type parent-from-marker nil)
> +      (push (cons parent-from-marker t) lst)
> +      (setq continue t))
> +    (when (and parent-to-marker
> +               (eq pos (marker-position parent-to-marker)))
> +      (set-marker-insertion-type parent-to-marker t)
> +      (push (cons parent-to-marker nil) lst)
> +      (setq continue t))
> +    (when continue
> +      (setq lst
> +            (append lst (widget--prepare-markers-for-inside-insertion parent))))
> +    lst))

It just occurred to me that if widget updates happen recursively
(meaning that another widget update happens between the two calls to
`widget--prepare-markers-for-*-insertion`), then the
`widget--prepare-markers-for-outside-insertion` done at the end
of the inner call will mean that any insertion that happens afterwards
can end up inserting "outside" instead of "inside".  So, I think to
protect against that, we need the above code to push only those markers
which we do change (i.e. we need to test the current value of
`marker-insertion-type` before pushing it to `lst`).
Does that make sense, or am I imagining things?


        Stefan


PS: While I'm here, I might as well mention the irrelevant tweaks
  I'd be tempted to do:

- Maybe instead of "prepare" I'd use "revert" in the name of the
  second function.
- `continue` is non-nil exactly when `lst` is non-nil, so we can drop
  `continue`.
- Since `lst` is a freshly consructed list, we can use `nconc`.
- The `setq` at the end is right before returning `lst`
  so we don't need it, instead we can finish with:

    (when lst
      (nconc lst (widget--prepare-markers-for-inside-insertion parent)))))

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.