{"thread":{"id":"6715","subject":"[PATCH] Make gitk save and restore the user set window position.","startedAt":"2007-02-07T03:21:05Z","lastAt":"2007-02-08T00:30:38Z","messageCount":3,"participants":["Junio C Hamano","Shawn O. Pearce","Mark Levedahl"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"33767","messageId":"7v3b5ifyam.fsf@assigned-by-dhcp.cox.net","threadId":"6715","inReplyTo":null,"subject":"[PATCH] Make gitk save and restore the user set window position.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-07T03:21:05Z","receivedAt":"2007-02-07T03:21:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Mark Levedahl <mdl123@verizon.net>\nDate: Mon, 05 Feb 2007 22:59:50 -0500\n\ngitk was saving widget sizes and positions when the main window was\ndestroyed, which is after all child widgets are destroyed. Tk resizes\ncontainer widgets as children are destroyed, so the saved main window\ninformation was not as the user left the window. This patch causes the\ninfo to be saved before the first child widget is destroyed. Also,\nuse wm geometry rather than winfo geometry (both work on Windows, on\nLinux they give different answers and the former is correct).\n\nSigned-off-by: Mark Levedahl <mdl123@verizon.net>\n---\n * Somehow this did not appeared on the list, so I am forwarding.\n\n   After seeing what this patch has to do, I feel dirty, but\n   that is not Mark's fault -- rather it is Tk's.\n\n   I am tempted to suggest adding an explicit \"Save window\n   configuration\" action on the menubar and forget about\n   resurrecting the window configuration immediately before the\n   end of the last session.\n\n gitk |   20 ++++++++++++++++++--\n 1 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 8132812..beec6a9 100755\n--- a/gitk\n+++ b/gitk\n@@ -725,7 +725,7 @@ proc makewindow {} {\n     bind . <Control-KP_Add> {incrfont 1}\n     bind . <Control-minus> {incrfont -1}\n     bind . <Control-KP_Subtract> {incrfont -1}\n-    bind . <Destroy> {savestuff %W}\n+    bindalldestroy .\n     bind . <Button-1> \"click %W\"\n     bind $fstring <Key-Return> dofind\n     bind $sha1entry <Key-Return> gotocommit\n@@ -759,6 +759,22 @@ proc makewindow {} {\n \t-command rmbranch\n }\n \n+# The main window resizes itself while the windows are being destroyed,\n+# we want to save the size before it is resized so we write out when the\n+# first widget with no children is destroyed.  We don't know which this\n+# will be, so we bind all child widgets (savestuff will save only once,\n+# the remaining calls are ignored).\n+proc bindalldestroy {w} {\n+    set children [winfo children $w]\n+    if {\"$children\" == \"\"} {\n+        bind $w <Destroy> {savestuff %W}\n+    } else {\n+        foreach child $children {\n+            bindalldestroy $child\n+        }\n+    }\n+}\n+\n # mouse-2 makes all windows scan vertically, but only the one\n # the cursor is in scans horizontally\n proc canvscan {op w x y} {\n@@ -829,7 +845,7 @@ proc savestuff {w} {\n \tputs $f [list set colors $colors]\n \tputs $f [list set diffcolors $diffcolors]\n \n-        puts $f \"set geometry(main) [winfo geometry .]\"\n+\tputs $f \"set geometry(main) [wm geometry .]\"\n \tputs $f \"set geometry(topwidth) [winfo width .tf]\"\n \tputs $f \"set geometry(topheight) [winfo height .tf]\"\n \tputs $f \"set geometry(canv) [expr {[winfo width $canv]-0}]\"\n-- \n1.5.0.rc3.24.g0c5e\n"},{"id":"33770","messageId":"20070207054010.GB7047@spearce.org","threadId":"6715","inReplyTo":"7v3b5ifyam.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Make gitk save and restore the user set window position.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-02-07T05:40:10Z","receivedAt":"2007-02-07T05:40:10Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n>    After seeing what this patch has to do, I feel dirty, but\n>    that is not Mark's fault -- rather it is Tk's.\n> \n>    I am tempted to suggest adding an explicit \"Save window\n>    configuration\" action on the menubar and forget about\n>    resurrecting the window configuration immediately before the\n>    end of the last session.\n\nMaybe take a look at what git-gui does here, because its slightly\nsaner and still saves the geometry on exit:\n\n\tset is_quitting 0\n\tproc do_quit {} {\n\t\tglobal is_quitting\n\t\tif {$is_quitting} return\n\t\tset is_quitting 1\n\t\t# save wm geometry\n\t}\n\tbind . <Destroy> do_quit\n\nOK, its not that much saner.  But it does bypass needing to setup\nsome ugly bindings on every object in the UI.  Though I recently\ntook a slightly different approach in a dialog:\n\n\tproc do_quit {} {\n\t\tbind . <Destroy> {}\n\t\t# do cleanup\n\t}\n\tbind . <Destroy> do_quit\n\nYes the binding is firing in both cases for some arbitrary child\nwidget in the window, but it doesn't matter.  In the latter version\nsetting the binding to the empty string removes the do_quit binding,\nallowing the other widgets to destroy without reinvoking do_quit\nand whacking whatever geometry data you may have saved before the\nwidgets started to get deleted.\n\n\nThe only problem I seem to have in git-gui is the window position\nopens about 10 pixels lower and 2 pixels to the right than the last\ntime it opened.  I think there's a bug in Tk on Windows where the\nwindow position on the desktop doesn't include the titlebar when I\nget it, but expects to include it when I attempt to set it on the\nnext start.\n\nI should note I also see the same behavior with my day-time-job's\nJava apps on Windows however, so I don't think its a specific Tk\nor git-gui issue.\n\n-- \nShawn.\n"},{"id":"33901","messageId":"45CA6F2E.7010707@verizon.net","threadId":"6715","inReplyTo":"20070207054010.GB7047@spearce.org","subject":"Re: [PATCH] Make gitk save and restore the user set window position.","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-02-08T00:30:38Z","receivedAt":"2007-02-08T00:30:38Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Shawn O. Pearce wrote:\n> Junio C Hamano <junkio@cox.net> wrote:\n>   \n>>    After seeing what this patch has to do, I feel dirty, but\n>>    that is not Mark's fault -- rather it is Tk's.\n>>\n>>    I am tempted to suggest adding an explicit \"Save window\n>>    configuration\" action on the menubar and forget about\n>>    resurrecting the window configuration immediately before the\n>>    end of the last session.\n>>     \n>\n> Maybe take a look at what git-gui does here, because its slightly\n> saner and still saves the geometry on exit:\n>\n> \tset is_quitting 0\n> \tproc do_quit {} {\n> \t\tglobal is_quitting\n> \t\tif {$is_quitting} return\n> \t\tset is_quitting 1\n> \t\t# save wm geometry\n> \t}\n> \tbind . <Destroy> do_quit\n>\n> OK, its not that much saner.  But it does bypass needing to setup\n> some ugly bindings on every object in the UI.  Though I recently\n> took a slightly different approach in a dialog:\n>\n> \tproc do_quit {} {\n> \t\tbind . <Destroy> {}\n> \t\t# do cleanup\n> \t}\n> \tbind . <Destroy> do_quit\n>\n> Yes the binding is firing in both cases for some arbitrary child\n> widget in the window, but it doesn't matter.  In the latter version\n> setting the binding to the empty string removes the do_quit binding,\n> allowing the other widgets to destroy without reinvoking do_quit\n> and whacking whatever geometry data you may have saved before the\n> widgets started to get deleted.\n>\n>\n> The only problem I seem to have in git-gui is the window position\n> opens about 10 pixels lower and 2 pixels to the right than the last\n> time it opened.  I think there's a bug in Tk on Windows where the\n> window position on the desktop doesn't include the titlebar when I\n> get it, but expects to include it when I attempt to set it on the\n> next start.\n>\n> I should note I also see the same behavior with my day-time-job's\n> Java apps on Windows however, so I don't think its a specific Tk\n> or git-gui issue.\n>\n>   \nWhat you suggest is essentially what gitk does before my patch. In \ngit-gui, routine do_quit is invoked when Tk is ready to destroy the top \nlevel window: this is _after_ Tk has destroyed everything in the window, \nand thus the problem. I still haven't isolated which object(s) being \ndestroyed from gitk cause the geometry reported from wm info to change, \nclearly my patch is binding widgets that do not need to be bound, but I \nhaven't found which specific one needs to be bound.\n\nWith my patch, the correct position is saved and restored on both Linux \nand Cygwin. I think that should be the goal. A more elegant solution \naccomplishing that end is of course desirable, but there remains the \nissue of finding it. I think the objections to my patch are more \ntheoretical than practical: I seriously doubt you can find any \npractically observable side effect of binding all the widgets other than \nthat the window geometry _is_ correctly saved and restored.\n\nAs to being specific to Tk, I have many windows applications that \nsuccessfully save and restore window state, this geometry issue is not \nendemic to Windows and I have never encountered it except with Tk.\n\nMark\n"}]}