[PATCH] gitk: restore wm state to normal before saving geometry information

gitk now includes patches for saving and restoring wm state, however
because it saves wm geometry when window can still be maximized the
maximize/restore button becomes useless after restarting gitk (you
will get a huge displaced window if you try to restore it). This
patch fixes this issue by storing window geometry in normal state.

Signed-off-by: Alexey Borzenkov <redacted>
---
 My previous email didn't come out rights, I hope this one will
 Don't forget to cc me if you have any comments/questions

 gitk |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index 8c08310..fedeb88 100755
--- a/gitk
+++ b/gitk

@@ -2555,8 +2555,11 @@ proc savestuff {w} {
 	puts $f [list set extdifftool $extdifftool]
 	puts $f [list set perfile_attrs $perfile_attrs]
 
-	puts $f "set geometry(main) [wm geometry .]"
 	puts $f "set geometry(state) [wm state .]"
+	if {[wm state .] eq {zoomed}} {
+		wm state . normal
+	}
+	puts $f "set geometry(main) [wm geometry .]"
 	puts $f "set geometry(topwidth) [winfo width .tf]"
 	puts $f "set geometry(topheight) [winfo height .tf]"
         puts $f "set geometry(pwsash0) \"[.tf.histframe.pwclist sash coord 0]\""

-- 
1.6.4.2

If a file exists and is hidden on Windows the Tcl open command will
fail as the attributes provided in the CREAT call fail to match those
of the existing file. Forcing removal of the temporary file before we
begin solves any problems caused by previous failures to save the
application settings. An alternative would be to remove the hidden
attribute before calling 'open'.

Signed-off-by: Pat Thoyts <redacted>
---
 gitk |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index 1306178..a0214b7 100755
--- a/gitk
+++ b/gitk

@@ -2526,6 +2526,7 @@ proc savestuff {w} {
     if {$stuffsaved} return
     if {![winfo viewable .]} return
     catch {
+	if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
 	set f [open "~/.gitk-new" w]
 	if {$::tcl_platform(platform) eq {windows}} {
 	    file attributes "~/.gitk-new" -hidden true

-- 
1.6.4.msysgit.0

The patch to handle the geometry of a restored gitk by Alexy Borzenkov
causes the position of the columns to creep each time the application
is restarted. This patch addresses this by remembering the application
geometry for the normal state and saving that regardless of the actual
state when the application is closed.

Signed-off-by: Pat Thoyts <redacted>
---
 gitk |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index a0214b7..67122c3 100755
--- a/gitk
+++ b/gitk

@@ -2251,6 +2251,8 @@ proc makewindow {} {
 	    }
 	    wm geometry . "${w}x$h"
 	}
+    } else {
+	set geometry(main) [wm geometry .]
     }
 
     if {[info exists geometry(state)] && $geometry(state) eq "zoomed"} {

@@ -2265,6 +2267,11 @@ proc makewindow {} {
         set ::BM "2"
     }
 
+    bind . <Configure> {
+	if {[winfo toplevel %W] eq "%W" && [wm state %W] eq "normal"} {
+	    set ::geometry(main) %wx%h+%x+%y
+	}
+    }
     bind .pwbottom <Configure> {resizecdetpanes %W %w}
     pack .ctop -fill both -expand 1
     bindall <1> {selcanvline %W %x %y}

@@ -2556,7 +2563,7 @@ proc savestuff {w} {
 	puts $f [list set extdifftool $extdifftool]
 	puts $f [list set perfile_attrs $perfile_attrs]
 
-	puts $f "set geometry(main) [wm geometry .]"
+	puts $f "set geometry(main) $::geometry(main)"
 	puts $f "set geometry(state) [wm state .]"
 	puts $f "set geometry(topwidth) [winfo width .tf]"
 	puts $f "set geometry(topheight) [winfo height .tf]"

-- 
1.6.4.msysgit.0

On Tue, Sep 15, 2009 at 4:03 PM, Pat Thoyts
[off-list ref] wrote:

quoted

gitk now includes patches for saving and restoring wm state, however
because it saves wm geometry when window can still be maximized the
maximize/restore button becomes useless after restarting gitk (you
will get a huge displaced window if you try to restore it). This
patch fixes this issue by storing window geometry in normal state.

I tried this patch on windows and I find that it causes the columns in
the top view to creep each time you restart the application. This is I
think due to the way this patch sets the state to normal before
recording all the settings.

This is strange, as I certainly don't see this behaviour now (I'm
using gitk in version 1.6.4.3). Actually, I did see that behaviour
once, but I believe this was when I ran an unpatched gitk which seemed
to record wrong geometry. As soon as I cleared ~/.gitk of those wrong
coordinates I haven't seen this behaviour anymore.

On the other hand, when I resize columns and then maximize/restore the
window repeatedly I see that their sizes change in a strange way (and
the smaller restored window they stranger are results) until hitting
some sort of equilibrium, then maximize/restore doesn't have effect on
their sizes anymore. So maybe there's a bug not in a way my patch
restores the window, but in a way window resizes are handled.

I will post an alternative patch that records the normal geometry
whenever it changes instead which seems to work better for me.

By all means.

On Tue, Sep 15, 2009 at 11:26, Pat Thoyts
[off-list ref] wrote:

maybe another gitk instance is writing it at exactly same moment
in time? Writing is known to take a few moments. Especially on Windows.

On Tue, Sep 15, 2009 at 4:54 PM, Alexey Borzenkov [off-list ref] wrote:

On the other hand, when I resize columns and then maximize/restore the
window repeatedly I see that their sizes change in a strange way (and
the smaller restored window they stranger are results) until hitting
some sort of equilibrium, then maximize/restore doesn't have effect on
their sizes anymore. So maybe there's a bug not in a way my patch
restores the window, but in a way window resizes are handled.

It seems that I found what's the problem. When the window is very
small, and we maximize it, sash coords would initially be small. Then
we calculate their new positions and set it with $win sash place, but
unfortunately, tk limits them to the OLD pane size for some reason.
Initially I though that maybe Configure is fired too early (before
window is resized), but [winfo width $win] shows $win has the new
size. Here's some output that I got during debugging:

resizing 422 -> 1261 (winfo shows 1261)
    s0: 138 1
    s1: 288 1
    sash0: 412 -> 412 1
    sash1: 860 -> 417 1

I'm at a loss why this happens and how to lift this restriction.

On Tue, Sep 15, 2009 at 1:37 PM, Pat Thoyts
[off-list ref] wrote:

The patch to handle the geometry of a restored gitk by Alexy Borzenkov
causes the position of the columns to creep each time the application
is restarted. This patch addresses this by remembering the application
geometry for the normal state and saving that regardless of the actual
state when the application is closed.

I can confirm it works even when the difference between normal and
maximized window are big.

On Tue, Sep 15, 2009 at 5:01 PM, Alex Riesen [off-list ref] wrote:

On Tue, Sep 15, 2009 at 11:26, Pat Thoyts
[off-list ref] wrote:

quoted

If a file exists and is hidden on Windows the Tcl open command will
fail as the attributes provided in the CREAT call fail to match those
of the existing file. Forcing removal of the temporary file before we
begin solves any problems caused by previous failures to save the
application settings. An alternative would be to remove the hidden
attribute before calling 'open'.

Signed-off-by: Pat Thoyts <redacted>
---
 gitk |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index 1306178..a0214b7 100755
--- a/gitk
+++ b/gitk

@@ -2526,6 +2526,7 @@ proc savestuff {w} {

    if {$stuffsaved} return
    if {![winfo viewable .]} return
    catch {
+       if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}

maybe another gitk instance is writing it at exactly same moment
in time? Writing is known to take a few moments. Especially on Windows.

Then deleting would fail, because on Windows opened files cannot be
deleted (unless they are opened in a special way that permits it).

Alex Riesen [off-list ref] writes:

On Tue, Sep 15, 2009 at 11:26, Pat Thoyts
[off-list ref] wrote:

quoted

If a file exists and is hidden on Windows the Tcl open command will
fail as the attributes provided in the CREAT call fail to match those
of the existing file. Forcing removal of the temporary file before we
begin solves any problems caused by previous failures to save the
application settings. An alternative would be to remove the hidden
attribute before calling 'open'.

Signed-off-by: Pat Thoyts <redacted>
---
 gitk |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index 1306178..a0214b7 100755
--- a/gitk
+++ b/gitk

@@ -2526,6 +2526,7 @@ proc savestuff {w} {

    if {$stuffsaved} return
    if {![winfo viewable .]} return
    catch {
+       if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}

maybe another gitk instance is writing it at exactly same moment
in time? Writing is known to take a few moments. Especially on Windows.

no - 'open $file w' is translated into O_WRONLY|O_CREAT|O_TRUNC
internally and passed to the win32 layer to get converted to a call to CreateFile with
GENERIC_WRITE, CREATE_ALWAYS and FILE_ATTRIBUTE_NORMAL. The file has
got FILE_ATTRIBUTE_HIDDEN though and as it exists and our attributes
do not match we get failed.
'open $file {O_WRONLY O_TRUNC}' would open it but we'd have to check
for non-existence and redo with O_CREAT if it was not already present.

See tclWinChan.c:TclpOpenFileChannel.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

Alexey Borzenkov [off-list ref] writes:

Then deleting would fail, because on Windows opened files cannot be
deleted (unless they are opened in a special way that permits it).

The delete occurs before we attempt to open the file which is why it
succeeds when such file is present.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

Alexey Borzenkov writes:

gitk now includes patches for saving and restoring wm state, however
because it saves wm geometry when window can still be maximized the
maximize/restore button becomes useless after restarting gitk (you
will get a huge displaced window if you try to restore it). This
patch fixes this issue by storing window geometry in normal state.

Hmmm, shouldn't we be also saving the window state (zoomed/normal) and
restoring that as well?

Paul.

Alexey Borzenkov [off-list ref] writes:

gitk now includes patches for saving and restoring wm state, however
because it saves wm geometry when window can still be maximized the
maximize/restore button becomes useless after restarting gitk (you
will get a huge displaced window if you try to restore it). This
patch fixes this issue by storing window geometry in normal state.

I tried this patch on windows and I find that it causes the columns in
the top view to creep each time you restart the application. This is I
think due to the way this patch sets the state to normal before
recording all the settings.

I will post an alternative patch that records the normal geometry
whenever it changes instead which seems to work better for me.

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

Paul Mackerras [off-list ref] writes:

quoted

patch fixes this issue by storing window geometry in normal state.

Hmmm, shouldn't we be also saving the window state (zoomed/normal) and
restoring that as well?

Paul.

We do. That got added in c876dbad

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

Pat Thoyts writes:

The patch to handle the geometry of a restored gitk by Alexy Borzenkov
causes the position of the columns to creep each time the application
is restarted.  This patch addresses this by remembering the application
geometry for the normal state and saving that regardless of the actual
state when the application is closed.

So this patch replaces Alexey's patch, then?  The context in your patch
doesn't match the changes made in Alexey's patch AFAICS.

Paul.

On Thu, Sep 17, 2009 at 4:15 PM, Paul Mackerras [off-list ref] wrote:

quoted

The patch to handle the geometry of a restored gitk by Alexy Borzenkov
causes the position of the columns to creep each time the application
is restarted.  This patch addresses this by remembering the application
geometry for the normal state and saving that regardless of the actual
state when the application is closed.

So this patch replaces Alexey's patch, then?  The context in your patch
doesn't match the changes made in Alexey's patch AFAICS.

Yes. Pat's patch replaces my patch.

Paul Mackerras [off-list ref] writes:

Pat Thoyts writes:

quoted

The patch to handle the geometry of a restored gitk by Alexy Borzenkov
causes the position of the columns to creep each time the application
is restarted.  This patch addresses this by remembering the application
geometry for the normal state and saving that regardless of the actual
state when the application is closed.

So this patch replaces Alexey's patch, then?  The context in your patch
doesn't match the changes made in Alexey's patch AFAICS.

Correct. I posted a response to Alexy's patch saying I'd post an
alternative as I had trouble with the columns resizing and creeping
due to the assertion of 'wm state normal' during the settings save
each time.
See
 http://thread.gmane.org/gmane.comp.version-control.git/128026/focus=128652
The context of mine should be the gitk repository master (was
c21398be).

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD