Subject: Re: bug#58175: 29.0.50; M-x window-swap-states during an active mark leaves behind a region overlay

From: Eli Zaretskii <[email-masked]>
Date: Wed, 05 Oct 2022 08:42:17 +0300

> From: Stefan Monnier <[email-masked]>
> Cc: martin rudalics <[email-masked]>,  [email-masked],
>   [email-masked]
> Date: Tue, 04 Oct 2022 17:25:13 -0400
> 
>       ;; Reset window's parameters and assign saved ones (we might want
>       ;; a `remove-window-parameters' function here).
>       (dolist (parameter (window-parameters window))
> 	(set-window-parameter window (car parameter) nil))
> 
> I don't think it's right to add `internal-region-overlay` to
> `window-persistent-parameters` since we don't want/need to store those
> overlays in window-state objects.
> 
> We could change the above code so it only sets to nil those
> parameters that are listed in `window-persistent-parameters`, but I'm
> not sure if that's the right choice.  It might be, tho: it seems odd to
> just zap properties owned by arbitrary packages without giving them
> a chance to "say goodbye".

Martin will tell, but I'm pretty sure this wasn't born out of thin
air.  I'm sure there are window parameters that will do harm if
copied.  Look at the list of window parameters in the "Window
Parameters" node of the ELisp manual, and try to convince yourself
that you want to copy all of them by default (we currently only copy
clone-of, AFAIU).

> Or we could add some kind of hook (similar to a `change-major-mode-hook`
> but for window state changes rather than major mode changes) so code
> like the region-highlight code can register itself there to throw away
> its overlays before a new window-state is installed.

Why is this cleaner than maintaining a list of "persistent"
parameters?

> Or we need to change the `redisplay--(un)highlight-overlay-function`s so
> as to keep their overlays (and similar info) elsewhere, probably in
> a variable rather than a window-parameter since window-parameters can
> disappear without warning.

Maybe.  But why complicate a mechanism that is already extremely
complicated and hard to understand for a bystander, and needed several
fixes until we got it right?  The use case in this bug report is
pretty marginal, so much so that from where I stand we could
legitimately say "this is not supported".  It hardly warrants making
questionable changes in mechanisms that are so central to routine
Emacs operation.

Btw, if we do want to consider changes in the region-overlay
machinery, then how about having
redisplay--unhighlight-overlay-function clean up by deleting overlays
whose buffer is nil or dead?  That would at least avoid leaving around
"stale" overlays that were once the region, which is what happens in
this case.