Subject: Re: bug#69941: 30.0.50; Faulty fontification of radio button widgets

From: Stefan Monnier <[email-masked]>
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)))))