{"thread":{"id":"64176","subject":"[PATCH v2 0/2] gitk: make the 'Tags and Heads' window geometry sticky","startedAt":"2025-09-20T18:40:16Z","lastAt":"2025-09-28T13:30:18Z","messageCount":8,"participants":["Michael Rappazzo","Johannes Sixt","Mark Levedahl","Mike Rappazzo"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"526864","messageId":"20250920184007.26183-1-rappazzo@gmail.com","threadId":"64176","inReplyTo":null,"subject":"[PATCH v2 0/2] gitk: make the 'Tags and Heads' window geometry sticky","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2025-09-20T18:40:05Z","receivedAt":"2025-09-20T18:40:16Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Differences from v1:\n - Add a fix to adjust the position of the main window on open.\n   Previously, the size was preserved, but not the position.\n - Simplified the mechanism for storing the size and position of the\n   tags and head view to use absolute positioning instead of being\n   relative to the parent window.\n\nMichael Rappazzo (2):\n  gitk: fix the position of the main main window on initialize\n  gitk: make Tags and Heads window geometry sticky\n\n gitk | 37 +++++++++++++++++++++++++++++++++++--\n 1 file changed, 35 insertions(+), 2 deletions(-)\n\n-- \n2.51.0\n\n"},{"id":"526865","messageId":"20250920184007.26183-2-rappazzo@gmail.com","threadId":"64176","inReplyTo":"20250920184007.26183-1-rappazzo@gmail.com","subject":"[PATCH v2 1/2] gitk: fix the position of the main main window on initialize","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2025-09-20T18:40:06Z","receivedAt":"2025-09-20T18:40:38Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"The main window geometry was only restoring size but not position.\nUse after idle to ensure proper timing on OS's where that is necessary.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n gitk | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/gitk b/gitk\nindex 6e4d71d585..95469a8fae 100755\n--- a/gitk\n+++ b/gitk\n@@ -2775,6 +2775,8 @@ proc makewindow {} {\n             }\n             wm geometry . \"${w}x$h\"\n         }\n+        # Restore full geometry including position after window is mapped\n+        after idle [list wm geometry . $geometry(main)]\n     }\n \n     if {[info exists geometry(state)] && $geometry(state) eq \"zoomed\"} {\n-- \n2.51.0\n\n"},{"id":"526866","messageId":"20250920184007.26183-3-rappazzo@gmail.com","threadId":"64176","inReplyTo":"20250920184007.26183-1-rappazzo@gmail.com","subject":"[PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2025-09-20T18:40:07Z","receivedAt":"2025-09-20T18:40:40Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Currently, the Tags and Heads window always opens at a default position\nand size, requiring users to reposition it each time. This patch makes\nthe window remember its geometry between sessions.\n\nThis change saves and restores the Tags and Heads window size and position\nrelative to the main gitk window. The geometry is stored in the config file\nas `geometry(showrefs)` and persists between gitk sessions. The window\nposition is stored relative to the main window, so it maintains the same\nspatial relationship when the main window is moved or when gitk is restarted\non different monitors.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n gitk | 35 +++++++++++++++++++++++++++++++++--\n 1 file changed, 33 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 95469a8fae..0393241c85 100755\n--- a/gitk\n+++ b/gitk\n@@ -3116,6 +3116,11 @@ proc savestuff {w} {\n         puts $f \"set geometry(pwsash1) \\\"[.tf.histframe.pwclist sashpos 1] 1\\\"\"\n         puts $f \"set geometry(botwidth) [winfo width .bleft]\"\n         puts $f \"set geometry(botheight) [winfo height .bleft]\"\n+        if {[winfo exists .showrefs]} {\n+            puts $f \"set geometry(showrefs) \\\"[wm geometry .showrefs]\\\"\"\n+        } elseif {[info exists geometry(showrefs)]} {\n+            puts $f \"set geometry(showrefs) \\\"$geometry(showrefs)\\\"\"\n+        }\n \n         array set view_save {}\n         array set views {}\n@@ -10209,11 +10214,13 @@ proc showrefs {} {\n     if {[winfo exists $top]} {\n         raise $top\n         refill_reflist\n+        wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]\n         return\n     }\n     ttk_toplevel $top\n     wm title $top [mc \"Tags and heads: %s\" [file tail [pwd]]]\n     make_transient $top .\n+    wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]\n     text $top.list -background $bgcolor -foreground $fgcolor \\\n         -selectbackground $selectbgcolor -font mainfont \\\n         -xscrollcommand \"$top.xsb set\" -yscrollcommand \"$top.ysb set\" \\\n@@ -10239,8 +10246,8 @@ proc showrefs {} {\n     ttk::checkbutton $top.sort -text [mc \"Sort refs by type\"] \\\n         -variable sortrefsbytype -command {refill_reflist}\n     grid $top.sort - -sticky w -pady 2\n-    ttk::button $top.close -command [list destroy $top] -text [mc \"Close\"]\n-    bind $top <Key-Escape> [list destroy $top]\n+    ttk::button $top.close -command [list destroy_showrefs $top] -text [mc \"Close\"]\n+    bind $top <Key-Escape> [list destroy_showrefs $top]\n     grid $top.close -\n     grid columnconfigure $top 0 -weight 1\n     grid rowconfigure $top 0 -weight 1\n@@ -10249,6 +10256,8 @@ proc showrefs {} {\n     bind $top.list <ButtonRelease-1> {sel_reflist %W %x %y; break}\n     set reflist {}\n     refill_reflist\n+    after idle [list manage_showrefs_geometry $top restore]\n+    bind $top <Configure> [list manage_showrefs_geometry $top save]\n }\n \n proc sel_reflist {w x y} {\n@@ -10281,6 +10290,28 @@ proc reflistfilter_change {n1 n2 op} {\n     after 200 refill_reflist\n }\n \n+proc manage_showrefs_geometry {top action} {\n+    global geometry\n+    switch $action {\n+        save {\n+            if {[winfo exists $top]} {\n+                set geometry(showrefs) [wm geometry $top]\n+            }\n+        }\n+        restore {\n+            if {[info exists geometry(showrefs)] && [winfo exists $top]} {\n+                after 1 [list wm geometry $top $geometry(showrefs)]\n+            }\n+        }\n+    }\n+}\n+\n+proc destroy_showrefs {top} {\n+    manage_showrefs_geometry $top save\n+    savestuff .\n+    destroy $top\n+}\n+\n proc refill_reflist {} {\n     global reflist reflistfilter showrefstop headids tagids otherrefids sortrefsbytype\n     global curview upstreamofref\n-- \n2.51.0\n\n"},{"id":"526914","messageId":"c6a33014-5d87-4750-b6ce-234e944131b4@kdbg.org","threadId":"64176","inReplyTo":"20250920184007.26183-3-rappazzo@gmail.com","subject":"Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-09-22T06:34:47Z","receivedAt":"2025-09-22T06:34:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 20.09.25 um 20:40 schrieb Michael Rappazzo:\n> Currently, the Tags and Heads window always opens at a default position\n> and size, requiring users to reposition it each time. This patch makes\n> the window remember its geometry between sessions.\n> \n> This change saves and restores the Tags and Heads window size and position\n> relative to the main gitk window. The geometry is stored in the config file\n\nThe \"relative to the main Gitk window\" is not true anymore.\n\n> as `geometry(showrefs)` and persists between gitk sessions. The window\n> position is stored relative to the main window, so it maintains the same\n> spatial relationship when the main window is moved or when gitk is restarted\n> on different monitors.\n> \n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n>  gitk | 35 +++++++++++++++++++++++++++++++++--\n>  1 file changed, 33 insertions(+), 2 deletions(-)\n> \n> diff --git a/gitk b/gitk\n> index 95469a8fae..0393241c85 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -3116,6 +3116,11 @@ proc savestuff {w} {\n>          puts $f \"set geometry(pwsash1) \\\"[.tf.histframe.pwclist sashpos 1] 1\\\"\"\n>          puts $f \"set geometry(botwidth) [winfo width .bleft]\"\n>          puts $f \"set geometry(botheight) [winfo height .bleft]\"\n> +        if {[winfo exists .showrefs]} {\n> +            puts $f \"set geometry(showrefs) \\\"[wm geometry .showrefs]\\\"\"\n> +        } elseif {[info exists geometry(showrefs)]} {\n> +            puts $f \"set geometry(showrefs) \\\"$geometry(showrefs)\\\"\"\n> +        }\n>  \n>          array set view_save {}\n>          array set views {}\n> @@ -10209,11 +10214,13 @@ proc showrefs {} {\n>      if {[winfo exists $top]} {\n>          raise $top\n>          refill_reflist\n> +        wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]\n>          return\n>      }\n>      ttk_toplevel $top\n>      wm title $top [mc \"Tags and heads: %s\" [file tail [pwd]]]\n>      make_transient $top .\n> +    wm protocol $top WM_DELETE_WINDOW [list destroy_showrefs $top]\n>      text $top.list -background $bgcolor -foreground $fgcolor \\\n>          -selectbackground $selectbgcolor -font mainfont \\\n>          -xscrollcommand \"$top.xsb set\" -yscrollcommand \"$top.ysb set\" \\\n> @@ -10239,8 +10246,8 @@ proc showrefs {} {\n>      ttk::checkbutton $top.sort -text [mc \"Sort refs by type\"] \\\n>          -variable sortrefsbytype -command {refill_reflist}\n>      grid $top.sort - -sticky w -pady 2\n> -    ttk::button $top.close -command [list destroy $top] -text [mc \"Close\"]\n> -    bind $top <Key-Escape> [list destroy $top]\n> +    ttk::button $top.close -command [list destroy_showrefs $top] -text [mc \"Close\"]\n> +    bind $top <Key-Escape> [list destroy_showrefs $top]\n>      grid $top.close -\n>      grid columnconfigure $top 0 -weight 1\n>      grid rowconfigure $top 0 -weight 1\n> @@ -10249,6 +10256,8 @@ proc showrefs {} {\n>      bind $top.list <ButtonRelease-1> {sel_reflist %W %x %y; break}\n>      set reflist {}\n>      refill_reflist\n> +    after idle [list manage_showrefs_geometry $top restore]\n\nMy thinking without having debugged it is:\n\n 1. A Configure event happens with the default geometry when the window\nbecomes visible. This records the default geometry in geometry(showrefs)\nby the handler that is bound in the next line below.\n\n 2. \"After idle\" the geometry is set to the then-current value of\ngeometry(showrefs), which would then be the default geometry and not the\none restored from the settings.\n\nWhy is it not necessary to encode the now-current value of\ngeometry(showrefs) (the restored value) in this after-idle handler? IOW,\nwhy does this work?\n\n> +    bind $top <Configure> [list manage_showrefs_geometry $top save]\n\nWith this binding, all size and position changes are immediately\nrecorded in the global geometry(showrefs) variable. Why do we still have\nto bind to so many other close events? Why is it necessary to check for\n`winfo exists .showrefs` and handle that in a separate branch in proc\nsavestuff above?\n\n>  }\n>  \n>  proc sel_reflist {w x y} {\n> @@ -10281,6 +10290,28 @@ proc reflistfilter_change {n1 n2 op} {\n>      after 200 refill_reflist\n>  }\n>  \n> +proc manage_showrefs_geometry {top action} {\n> +    global geometry\n> +    switch $action {\n> +        save {\n> +            if {[winfo exists $top]} {\n> +                set geometry(showrefs) [wm geometry $top]\n> +            }\n> +        }\n> +        restore {\n> +            if {[info exists geometry(showrefs)] && [winfo exists $top]} {\n> +                after 1 [list wm geometry $top $geometry(showrefs)]\n> +            }\n> +        }\n> +    }\n> +}\n\nThe two branches have no common code path. What is the rationale to have\na single function with sub-commands instead of two distinct functions?\n\n> +\n> +proc destroy_showrefs {top} {\n> +    manage_showrefs_geometry $top save\n> +    savestuff .\n> +    destroy $top\n> +}\n> +\n>  proc refill_reflist {} {\n>      global reflist reflistfilter showrefstop headids tagids otherrefids sortrefsbytype\n>      global curview upstreamofref\n\n-- Hannes\n\n"},{"id":"526916","messageId":"199b7665-910a-4f44-a734-ced99bc8cb81@kdbg.org","threadId":"64176","inReplyTo":"20250920184007.26183-2-rappazzo@gmail.com","subject":"Re: [PATCH v2 1/2] gitk: fix the position of the main main window on initialize","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-09-22T06:00:24Z","receivedAt":"2025-09-22T06:41:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 20.09.25 um 20:40 schrieb Michael Rappazzo:\n> The main window geometry was only restoring size but not position.\n> Use after idle to ensure proper timing on OS's where that is necessary.\n> \n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n>  gitk | 2 ++\n>  1 file changed, 2 insertions(+)\n> \n> diff --git a/gitk b/gitk\n> index 6e4d71d585..95469a8fae 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -2775,6 +2775,8 @@ proc makewindow {} {\n>              }\n>              wm geometry . \"${w}x$h\"\n>          }\n> +        # Restore full geometry including position after window is mapped\n> +        after idle [list wm geometry . $geometry(main)]\n>      }\n>  \n>      if {[info exists geometry(state)] && $geometry(state) eq \"zoomed\"} {\n\nI have been carrying 22d37f865268 (\"Revert \"gitk: Only restore window\nsize from ~/.gitk, not position\"\", 2008-05-26) since, like, 17 years in\nmy branch j6t-testing. Perhaps Mark can tell us why b9bee11526ec (\"gitk:\nOnly restore window size from ~/.gitk, not position\", 2008-03-10) was\nneeded...\n\n-- Hannes\n\n"},{"id":"526920","messageId":"7995c79c-b763-4a6e-830b-fbe29bf252f5@gmail.com","threadId":"64176","inReplyTo":"199b7665-910a-4f44-a734-ced99bc8cb81@kdbg.org","subject":"Re: [PATCH v2 1/2] gitk: fix the position of the main main window on initialize","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2025-09-22T10:16:09Z","receivedAt":"2025-09-22T10:16:11Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"No longer relevant. Cygwin up until 2011 used an unsupportable port of the Windows Tcl/Tk\npermanently stuck at 8.4.1. 8.4.1 has some bad bugs in its layout engine, and forced\nchanges in gitk to be compatible. All this became irrelevant around 2011 after Cygwin\ngained an X11 server and switched to a supportable port of the Unix/X11 Tcl/Tk (it is now\non the current 8.6 code base).\n\nOn 9/22/25 2:00 AM, Johannes Sixt wrote:\n> Am 20.09.25 um 20:40 schrieb Michael Rappazzo:\n>> The main window geometry was only restoring size but not position.\n>> Use after idle to ensure proper timing on OS's where that is necessary.\n>>\n>> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n>> ---\n>>  gitk | 2 ++\n>>  1 file changed, 2 insertions(+)\n>>\n>> diff --git a/gitk b/gitk\n>> index 6e4d71d585..95469a8fae 100755\n>> --- a/gitk\n>> +++ b/gitk\n>> @@ -2775,6 +2775,8 @@ proc makewindow {} {\n>>              }\n>>              wm geometry . \"${w}x$h\"\n>>          }\n>> +        # Restore full geometry including position after window is mapped\n>> +        after idle [list wm geometry . $geometry(main)]\n>>      }\n>>  \n>>      if {[info exists geometry(state)] && $geometry(state) eq \"zoomed\"} {\n> I have been carrying 22d37f865268 (\"Revert \"gitk: Only restore window\n> size from ~/.gitk, not position\"\", 2008-05-26) since, like, 17 years in\n> my branch j6t-testing. Perhaps Mark can tell us why b9bee11526ec (\"gitk:\n> Only restore window size from ~/.gitk, not position\", 2008-03-10) was\n> needed...\n>\n> -- Hannes\n>\n\n"},{"id":"527310","messageId":"CANoM8SW6gsfmhPYWq2_7f9DuwyQ4vVpbWkaPn4mDTg--LAZUJg@mail.gmail.com","threadId":"64176","inReplyTo":"c6a33014-5d87-4750-b6ce-234e944131b4@kdbg.org","subject":"Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2025-09-25T12:45:52Z","receivedAt":"2025-09-25T12:46:06Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Mon, Sep 22, 2025 at 2:34 AM Johannes Sixt <j6t@kdbg.org> wrote:\n> > @@ -10249,6 +10256,8 @@ proc showrefs {} {\n> >      bind $top.list <ButtonRelease-1> {sel_reflist %W %x %y; break}\n> >      set reflist {}\n> >      refill_reflist\n> > +    after idle [list manage_showrefs_geometry $top restore]\n>\n> My thinking without having debugged it is:\n>\n>  1. A Configure event happens with the default geometry when the window\n> becomes visible. This records the default geometry in geometry(showrefs)\n> by the handler that is bound in the next line below.\n>\n>  2. \"After idle\" the geometry is set to the then-current value of\n> geometry(showrefs), which would then be the default geometry and not the\n> one restored from the settings.\n>\n> Why is it not necessary to encode the now-current value of\n> geometry(showrefs) (the restored value) in this after-idle handler? IOW,\n> why does this work?\n\nWhen I was testing this, I used MacOS, Windows 11, and Gnome (Ubuntu).\nOn Mac the call\nworked without the `after idle`.  On both Windows and Gnome, it needed\nthe `after idle` for it\nto work as I expected.  I'm not sure exactly why.  Do you want me to\ntry to adjust this?  Do you\nhave a suggestion for it?\n\n\n> > +proc manage_showrefs_geometry {top action} {\n> > +    global geometry\n> > +    switch $action {\n> > +        save {\n> > +            if {[winfo exists $top]} {\n> > +                set geometry(showrefs) [wm geometry $top]\n> > +            }\n> > +        }\n> > +        restore {\n> > +            if {[info exists geometry(showrefs)] && [winfo exists $top]} {\n> > +                after 1 [list wm geometry $top $geometry(showrefs)]\n> > +            }\n> > +        }\n> > +    }\n> > +}\n>\n> The two branches have no common code path. What is the rationale to have\n> a single function with sub-commands instead of two distinct functions?\n\nYeah, that's my bad.  I started with something different, and whittled\nit down to this.  I'll adjust\nin the next iteration.\n\n_Mike\n"},{"id":"527504","messageId":"CANoM8SXnxxF6UMSfQ06ANfEv7HFCEEMCVoGgod1-DuFeHp6tXg@mail.gmail.com","threadId":"64176","inReplyTo":"CANoM8SW6gsfmhPYWq2_7f9DuwyQ4vVpbWkaPn4mDTg--LAZUJg@mail.gmail.com","subject":"Re: [PATCH v2 2/2] gitk: make Tags and Heads window geometry sticky","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2025-09-28T13:30:05Z","receivedAt":"2025-09-28T13:30:18Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Thu, Sep 25, 2025 at 8:45 AM Mike Rappazzo <rappazzo@gmail.com> wrote:\n>\n> On Mon, Sep 22, 2025 at 2:34 AM Johannes Sixt <j6t@kdbg.org> wrote:\n> > > @@ -10249,6 +10256,8 @@ proc showrefs {} {\n> > >      bind $top.list <ButtonRelease-1> {sel_reflist %W %x %y; break}\n> > >      set reflist {}\n> > >      refill_reflist\n> > > +    after idle [list manage_showrefs_geometry $top restore]\n> >\n> > My thinking without having debugged it is:\n> >\n> >  1. A Configure event happens with the default geometry when the window\n> > becomes visible. This records the default geometry in geometry(showrefs)\n> > by the handler that is bound in the next line below.\n> >\n> >  2. \"After idle\" the geometry is set to the then-current value of\n> > geometry(showrefs), which would then be the default geometry and not the\n> > one restored from the settings.\n> >\n> > Why is it not necessary to encode the now-current value of\n> > geometry(showrefs) (the restored value) in this after-idle handler? IOW,\n> > why does this work?\n>\n> When I was testing this, I used MacOS, Windows 11, and Gnome (Ubuntu).\n> On Mac the call\n> worked without the `after idle`.  On both Windows and Gnome, it needed\n> the `after idle` for it\n> to work as I expected.  I'm not sure exactly why.  Do you want me to\n> try to adjust this?  Do you\n> have a suggestion for it?\n>\n\nDigging into this a little more, I think the platform differences relate to\nhow each window manager handles the initial window mapping and geometry\nsetting sequence.\n\nOn MacOS, the window geometry can be set immediately during window creation\nwithout timing issues. However, on Windows and Gnome, there seems to\nbe a race condition where setting geometry too early gets overridden by\nthe window manager's default placement logic.\n\nUsing `after idle` ensures we set the geometry after the window manager\nhas finished its initial setup, which is why it's needed on Windows and\nGnome but not MacOS.\n\nI will split the manage_showrefs_geometry function and send an updated patch\n\n _Mike\n"}]}