From: Mike Rappazzo <rappazzo@gmail.com>
Date: Thu, 25 Sep 2025 12:45:52 GMT
Subject: Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky
Message-ID: <CANoM8SW6gsfmhPYWq2_7f9DuwyQ4vVpbWkaPn4mDTg--LAZUJg@mail.gmail.com>
In-Reply-To: <c6a33014-5d87-4750-b6ce-234e944131b4@kdbg.org>

On Mon, Sep 22, 2025 at 2:34 AM Johannes Sixt <j6t@kdbg.org> wrote:
> > @@ -10249,6 +10256,8 @@ proc showrefs {} {
> >      bind $top.list <ButtonRelease-1> {sel_reflist %W %x %y; break}
> >      set reflist {}
> >      refill_reflist
> > +    after idle [list manage_showrefs_geometry $top restore]
>
> My thinking without having debugged it is:
>
>  1. A Configure event happens with the default geometry when the window
> becomes visible. This records the default geometry in geometry(showrefs)
> by the handler that is bound in the next line below.
>
>  2. "After idle" the geometry is set to the then-current value of
> geometry(showrefs), which would then be the default geometry and not the
> one restored from the settings.
>
> Why is it not necessary to encode the now-current value of
> geometry(showrefs) (the restored value) in this after-idle handler? IOW,
> why does this work?

When I was testing this, I used MacOS, Windows 11, and Gnome (Ubuntu).
On Mac the call
worked without the `after idle`.  On both Windows and Gnome, it needed
the `after idle` for it
to work as I expected.  I'm not sure exactly why.  Do you want me to
try to adjust this?  Do you
have a suggestion for it?


> > +proc manage_showrefs_geometry {top action} {
> > +    global geometry
> > +    switch $action {
> > +        save {
> > +            if {[winfo exists $top]} {
> > +                set geometry(showrefs) [wm geometry $top]
> > +            }
> > +        }
> > +        restore {
> > +            if {[info exists geometry(showrefs)] && [winfo exists $top]} {
> > +                after 1 [list wm geometry $top $geometry(showrefs)]
> > +            }
> > +        }
> > +    }
> > +}
>
> The two branches have no common code path. What is the rationale to have
> a single function with sub-commands instead of two distinct functions?

Yeah, that's my bad.  I started with something different, and whittled
it down to this.  I'll adjust
in the next iteration.

_Mike