[PATCH v2 0/2] gitk: make the 'Tags and Heads' window geometry sticky

[PATCH v2 0/2] gitk: make the 'Tags and Heads' window geometry sticky

[Michael Rappazzo]

Differences from v1:
 - Add a fix to adjust the position of the main window on open.
   Previously, the size was preserved, but not the position.
 - Simplified the mechanism for storing the size and position of the
   tags and head view to use absolute positioning instead of being
   relative to the parent window.

Michael Rappazzo (2):
  gitk: fix the position of the main main window on initialize
  gitk: make Tags and Heads window geometry sticky

 gitk | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

-- 
2.51.0

[PATCH v2 1/2] gitk: fix the position of the main main window on initialize

[Michael Rappazzo]

The main window geometry was only restoring size but not position.
Use after idle to ensure proper timing on OS's where that is necessary.

Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
---
 gitk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gitk b/gitk
index 6e4d71d585..95469a8fae 100755
--- a/gitk
+++ b/gitk
@@ -2775,6 +2775,8 @@ proc makewindow {} {
             }
             wm geometry . "${w}x$h"
         }
+        # Restore full geometry including position after window is mapped
+        after idle [list wm geometry . $geometry(main)]
     }
 
     if {[info exists geometry(state)] && $geometry(state) eq "zoomed"} {
-- 
2.51.0

Re: [PATCH v2 1/2] gitk: fix the position of the main main window on initialize

[Johannes Sixt]

Am 20.09.25 um 20:40 schrieb Michael Rappazzo:
> The main window geometry was only restoring size but not position.
> Use after idle to ensure proper timing on OS's where that is necessary.
> 
> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> ---
>  gitk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/gitk b/gitk
> index 6e4d71d585..95469a8fae 100755
> --- a/gitk
> +++ b/gitk
> @@ -2775,6 +2775,8 @@ proc makewindow {} {
>              }
>              wm geometry . "${w}x$h"
>          }
> +        # Restore full geometry including position after window is mapped
> +        after idle [list wm geometry . $geometry(main)]
>      }
>  
>      if {[info exists geometry(state)] && $geometry(state) eq "zoomed"} {

I have been carrying 22d37f865268 ("Revert "gitk: Only restore window
size from ~/.gitk, not position"", 2008-05-26) since, like, 17 years in
my branch j6t-testing. Perhaps Mark can tell us why b9bee11526ec ("gitk:
Only restore window size from ~/.gitk, not position", 2008-03-10) was
needed...

-- Hannes

Re: [PATCH v2 1/2] gitk: fix the position of the main main window on initialize

[Mark Levedahl]

No longer relevant. Cygwin up until 2011 used an unsupportable port of the Windows Tcl/Tk
permanently stuck at 8.4.1. 8.4.1 has some bad bugs in its layout engine, and forced
changes in gitk to be compatible. All this became irrelevant around 2011 after Cygwin
gained an X11 server and switched to a supportable port of the Unix/X11 Tcl/Tk (it is now
on the current 8.6 code base).

On 9/22/25 2:00 AM, Johannes Sixt wrote:
> Am 20.09.25 um 20:40 schrieb Michael Rappazzo:
>> The main window geometry was only restoring size but not position.
>> Use after idle to ensure proper timing on OS's where that is necessary.
>>
>> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
>> ---
>>  gitk | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/gitk b/gitk
>> index 6e4d71d585..95469a8fae 100755
>> --- a/gitk
>> +++ b/gitk
>> @@ -2775,6 +2775,8 @@ proc makewindow {} {
>>              }
>>              wm geometry . "${w}x$h"
>>          }
>> +        # Restore full geometry including position after window is mapped
>> +        after idle [list wm geometry . $geometry(main)]
>>      }
>>  
>>      if {[info exists geometry(state)] && $geometry(state) eq "zoomed"} {
> I have been carrying 22d37f865268 ("Revert "gitk: Only restore window
> size from ~/.gitk, not position"", 2008-05-26) since, like, 17 years in
> my branch j6t-testing. Perhaps Mark can tell us why b9bee11526ec ("gitk:
> Only restore window size from ~/.gitk, not position", 2008-03-10) was
> needed...
>
> -- Hannes
>

[PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky

[Michael Rappazzo]

Currently, the Tags and Heads window always opens at a default position
and size, requiring users to reposition it each time. This patch makes
the window remember its geometry between sessions.

This change saves and restores the Tags and Heads window size and position
relative to the main gitk window. The geometry is stored in the config file
as `geometry(showrefs)` and persists between gitk sessions. The window
position is stored relative to the main window, so it maintains the same
spatial relationship when the main window is moved or when gitk is restarted
on different monitors.

Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
---
 gitk | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 95469a8fae..0393241c85 100755
--- a/gitk
+++ b/gitk
@@ -3116,6 +3116,11 @@ proc savestuff {w} {
         puts $f "set geometry(pwsash1) \"[.tf.histframe.pwclist sashpos 1] 1\""
         puts $f "set geometry(botwidth) [winfo width .bleft]"
         puts $f "set geometry(botheight) [winfo height .bleft]"
+        if {[winfo exists .showrefs]} {
+            puts $f "set geometry(showrefs) \"[wm geometry .showrefs]\""
+        } elseif {[info exists geometry(showrefs)]} {
+            puts $f "set geometry(showrefs) \"$geometry(showrefs)\""
+        }
 
         array set view_save {}
         array set views {}
@@ -10209,11 +10214,13 @@ proc showrefs {} {
     if {[winfo exists $top]} {
         raise $top
         refill_reflist
+        wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]
         return
     }
     ttk_toplevel $top
     wm title $top [mc "Tags and heads: %s" [file tail [pwd]]]
     make_transient $top .
+    wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]
     text $top.list -background $bgcolor -foreground $fgcolor \
         -selectbackground $selectbgcolor -font mainfont \
         -xscrollcommand "$top.xsb set" -yscrollcommand "$top.ysb set" \
@@ -10239,8 +10246,8 @@ proc showrefs {} {
     ttk::checkbutton $top.sort -text [mc "Sort refs by type"] \
         -variable sortrefsbytype -command {refill_reflist}
     grid $top.sort - -sticky w -pady 2
-    ttk::button $top.close -command [list destroy $top] -text [mc "Close"]
-    bind $top <Key-Escape> [list destroy $top]
+    ttk::button $top.close -command [list destroy_showrefs $top] -text [mc "Close"]
+    bind $top <Key-Escape> [list destroy_showrefs $top]
     grid $top.close -
     grid columnconfigure $top 0 -weight 1
     grid rowconfigure $top 0 -weight 1
@@ -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]
+    bind $top <Configure> [list manage_showrefs_geometry $top save]
 }
 
 proc sel_reflist {w x y} {
@@ -10281,6 +10290,28 @@ proc reflistfilter_change {n1 n2 op} {
     after 200 refill_reflist
 }
 
+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)]
+            }
+        }
+    }
+}
+
+proc destroy_showrefs {top} {
+    manage_showrefs_geometry $top save
+    savestuff .
+    destroy $top
+}
+
 proc refill_reflist {} {
     global reflist reflistfilter showrefstop headids tagids otherrefids sortrefsbytype
     global curview upstreamofref
-- 
2.51.0

Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky

[Johannes Sixt]

Am 20.09.25 um 20:40 schrieb Michael Rappazzo:
> Currently, the Tags and Heads window always opens at a default position
> and size, requiring users to reposition it each time. This patch makes
> the window remember its geometry between sessions.
> 
> This change saves and restores the Tags and Heads window size and position
> relative to the main gitk window. The geometry is stored in the config file

The "relative to the main Gitk window" is not true anymore.

> as `geometry(showrefs)` and persists between gitk sessions. The window
> position is stored relative to the main window, so it maintains the same
> spatial relationship when the main window is moved or when gitk is restarted
> on different monitors.
> 
> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> ---
>  gitk | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/gitk b/gitk
> index 95469a8fae..0393241c85 100755
> --- a/gitk
> +++ b/gitk
> @@ -3116,6 +3116,11 @@ proc savestuff {w} {
>          puts $f "set geometry(pwsash1) \"[.tf.histframe.pwclist sashpos 1] 1\""
>          puts $f "set geometry(botwidth) [winfo width .bleft]"
>          puts $f "set geometry(botheight) [winfo height .bleft]"
> +        if {[winfo exists .showrefs]} {
> +            puts $f "set geometry(showrefs) \"[wm geometry .showrefs]\""
> +        } elseif {[info exists geometry(showrefs)]} {
> +            puts $f "set geometry(showrefs) \"$geometry(showrefs)\""
> +        }
>  
>          array set view_save {}
>          array set views {}
> @@ -10209,11 +10214,13 @@ proc showrefs {} {
>      if {[winfo exists $top]} {
>          raise $top
>          refill_reflist
> +        wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]
>          return
>      }
>      ttk_toplevel $top
>      wm title $top [mc "Tags and heads: %s" [file tail [pwd]]]
>      make_transient $top .
> +    wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]
>      text $top.list -background $bgcolor -foreground $fgcolor \
>          -selectbackground $selectbgcolor -font mainfont \
>          -xscrollcommand "$top.xsb set" -yscrollcommand "$top.ysb set" \
> @@ -10239,8 +10246,8 @@ proc showrefs {} {
>      ttk::checkbutton $top.sort -text [mc "Sort refs by type"] \
>          -variable sortrefsbytype -command {refill_reflist}
>      grid $top.sort - -sticky w -pady 2
> -    ttk::button $top.close -command [list destroy $top] -text [mc "Close"]
> -    bind $top <Key-Escape> [list destroy $top]
> +    ttk::button $top.close -command [list destroy_showrefs $top] -text [mc "Close"]
> +    bind $top <Key-Escape> [list destroy_showrefs $top]
>      grid $top.close -
>      grid columnconfigure $top 0 -weight 1
>      grid rowconfigure $top 0 -weight 1
> @@ -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?

> +    bind $top <Configure> [list manage_showrefs_geometry $top save]

With this binding, all size and position changes are immediately
recorded in the global geometry(showrefs) variable. Why do we still have
to bind to so many other close events? Why is it necessary to check for
`winfo exists .showrefs` and handle that in a separate branch in proc
savestuff above?

>  }
>  
>  proc sel_reflist {w x y} {
> @@ -10281,6 +10290,28 @@ proc reflistfilter_change {n1 n2 op} {
>      after 200 refill_reflist
>  }
>  
> +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?

> +
> +proc destroy_showrefs {top} {
> +    manage_showrefs_geometry $top save
> +    savestuff .
> +    destroy $top
> +}
> +
>  proc refill_reflist {} {
>      global reflist reflistfilter showrefstop headids tagids otherrefids sortrefsbytype
>      global curview upstreamofref

-- Hannes

Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky

[Mike Rappazzo]

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

Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky

[Mike Rappazzo]

On Thu, Sep 25, 2025 at 8:45 AM Mike Rappazzo <rappazzo@gmail.com> wrote:
>
> 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?
>

Digging into this a little more, I think the platform differences relate to
how each window manager handles the initial window mapping and geometry
setting sequence.

On MacOS, the window geometry can be set immediately during window creation
without timing issues. However, on Windows and Gnome, there seems to
be a race condition where setting geometry too early gets overridden by
the window manager's default placement logic.

Using `after idle` ensures we set the geometry after the window manager
has finished its initial setup, which is why it's needed on Windows and
Gnome but not MacOS.

I will split the manage_showrefs_geometry function and send an updated patch

 _Mike