Bug in gitk - can post the window off screen

> Just a quick heads-up: I was running a dual screen setup with gitk on my 
> 2nd monitor. When I later started gitk up without the 2nd monitor, the 
> gitk window didn't appear. Given the time between the two sessions, it 
> didn't occur to me what the problem was.
> 
> Gitk should probably validate the "set geometry(main)" variable against 
> the current resolution.

> Tommy Thorn <tommy-git@thorn.ws> wrote:
> > Just a quick heads-up: I was running a dual screen setup with gitk on my 
> > 2nd monitor. When I later started gitk up without the 2nd monitor, the 
> > gitk window didn't appear. Given the time between the two sessions, it 
> > didn't occur to me what the problem was.
> > 
> > Gitk should probably validate the "set geometry(main)" variable against 
> > the current resolution.
> 
> Sad to say but this is a known issue with both gitk and git-gui.
> Both applications save the prior geometry and restore it during
> the next run, but neither validates the new geometry makes sense
> with the current available desktop space.  :-\

> torsdagen den 7 februari 2008 skrev Shawn O. Pearce:
> > Tommy Thorn <tommy-git@thorn.ws> wrote:
> > > Just a quick heads-up: I was running a dual screen setup with gitk on my 
> > > 2nd monitor. When I later started gitk up without the 2nd monitor, the 
> > > gitk window didn't appear. Given the time between the two sessions, it 
> > > didn't occur to me what the problem was.
> > > 
> > > Gitk should probably validate the "set geometry(main)" variable against 
> > > the current resolution.
> > 
> > Sad to say but this is a known issue with both gitk and git-gui.
> > Both applications save the prior geometry and restore it during
> > the next run, but neither validates the new geometry makes sense
> > with the current available desktop space.  :-\
> 
> Seems KDE forces it back into visible space, but it is annoying that the 
> gitk/git-gui windows do not come up on the same monitor as my cursor is on. 
> Could we change it and save just size, just like "all other" apps?

> lördagen den 23 februari 2008 skrev Gerrit Pape:
>
> > Saving the geometry can cause several inconveniences, e.g. when using a
>  > temporary dual screen setup, or sharing ~/.gitk between multiple
>  > systems.  Additionally it can be argued that window placement and sizing
>  > are the tasks of the window manager.  So don't do that anymore.
>
>  Shouldn't the restoring code also be dropped then?

> Gerrit Pape writes:
>
>   
>> Saving the geometry can cause several inconveniences, e.g. when using a
>> temporary dual screen setup, or sharing ~/.gitk between multiple
>> systems.  Additionally it can be argued that window placement and sizing
>> are the tasks of the window manager.  So don't do that anymore.
>>     
>
> I myself find it useful to have gitk remember the size and layout of
> the various panes, so I won't be applying this patch.
>
> The saving/restoring of the window position was done by Mark Levedahl
> to make gitk work properly on Cygwin.  I agree it can be annoying on X
> and I would take a patch to inhibit the restoring of the window
> position when running under X.  I'd want an ack from Mark before
> making that change more generally.
>
> Paul.
>
>   

> I can only offer to test the proposed patch, I am unable to predict what 
> will or won't trip up the buggy geometry engine in Cygwin's Tk. I do 
> remember I had trouble finding a way to restore the size but not the 
> position of the main window on Cygwin: that doesn't mean it can't be 
> done (logically, it should be easy), just that I didn't find the right 
> spell or incantation that would work. I'll also admit to not having 
> tried after I got to a completely working geometry solution, so perhaps 
> just restoring the size without position will now work on Cygwin.
> 
> Using the saved panel sizes in conjunction with the default window size 
> yields an unusable screen: many elements are obscured. The same occurs 
> using the defaults altogether: either way the user must resize and 
> adjust things to get to a workable layout. So, I am very opposed to 
> disabling the memory altogether. Also, I have a number of X apps that 
> remember their layout, so gitk's current behavior is not (at least to 
> me) an aberration and I would like that behavior to at least remain an 
> option.

> Mark Levedahl writes:
>
>  > I can only offer to test the proposed patch, I am unable to predict what
>  > will or won't trip up the buggy geometry engine in Cygwin's Tk. I do
>  > remember I had trouble finding a way to restore the size but not the
>  > position of the main window on Cygwin: that doesn't mean it can't be
>  > done (logically, it should be easy), just that I didn't find the right
>  > spell or incantation that would work. I'll also admit to not having
>  > tried after I got to a completely working geometry solution, so perhaps
>  > just restoring the size without position will now work on Cygwin.
>  >
>  > Using the saved panel sizes in conjunction with the default window size
>  > yields an unusable screen: many elements are obscured. The same occurs
>  > using the defaults altogether: either way the user must resize and
>  > adjust things to get to a workable layout. So, I am very opposed to
>  > disabling the memory altogether. Also, I have a number of X apps that
>  > remember their layout, so gitk's current behavior is not (at least to
>  > me) an aberration and I would like that behavior to at least remain an
>  > option.
>
>  Here's a patch for people to test.  It only restores the width and
>  height, and limits the width and height to be at most the width and
>  height of the screen.  It seems to work fine under X; I would be
>  interested to know what happens under macos and windows.
>
>  Paul.
>  ---
>  diff --git a/gitk b/gitk
>  index f1f21e9..f8f006f 100755
>  --- a/gitk
>  +++ b/gitk
>  @@ -930,9 +930,17 @@ proc makewindow {} {
>      .pwbottom add .bright
>      .ctop add .pwbottom
>
>  -    # restore window position if known
>  +    # restore window width & height if known
>      if {[info exists geometry(main)]} {
>  -        wm geometry . "$geometry(main)"
>  +       if {[scan $geometry(main) "%dx%d" w h] >= 2} {
>  +           if {$w > [winfo screenwidth .]} {
>  +               set w [winfo screenwidth .]
>  +           }
>  +           if {$h > [winfo screenheight .]} {
>  +               set h [winfo screenheight .]
>  +           }
>  +           wm geometry . "${w}x$h"
>  +       }
>      }
>
>      if {[tk windowingsystem] eq {aqua}} {
>
>  --
>

> This almost works on OS X.  The only problem is that it's not taking
> into account menubars and windowbars.  eg, if I have a screen
> resolution of 640x480, setting the window _content_ to that size will
> make the entire window including its titlebar about 500px tall.  Not
> only that, but the system-wide menubar at the top of the screen also
> reduces the available space, meaning that the maximum window size
> ought to be around 640x456.  (If you wanted to get even pickier, that
> will changing depending on the interface resolution...)

> Here's a patch for people to test.  It only restores the width and
> height, and limits the width and height to be at most the width and
> height of the screen.  It seems to work fine under X; I would be
> interested to know what happens under macos and windows.
>
> Paul.
> ---
> diff --git a/gitk b/gitk
> index f1f21e9..f8f006f 100755
> --- a/gitk
> +++ b/gitk
> @@ -930,9 +930,17 @@ proc makewindow {} {
>      .pwbottom add .bright
>      .ctop add .pwbottom
>  
> -    # restore window position if known
> +    # restore window width & height if known
>      if {[info exists geometry(main)]} {
> -        wm geometry . "$geometry(main)"
> +	if {[scan $geometry(main) "%dx%d" w h] >= 2} {
> +	    if {$w > [winfo screenwidth .]} {
> +		set w [winfo screenwidth .]
> +	    }
> +	    if {$h > [winfo screenheight .]} {
> +		set h [winfo screenheight .]
> +	    }
> +	    wm geometry . "${w}x$h"
> +	}
>      }
>  
>      if {[tk windowingsystem] eq {aqua}} {
>   

> I can confirm that the behaviour is much better: the window size is
> restored, as is the position, but it gives preference to the
> Xinerama head. You'd think it restores at the same *relative*
> position on the current head, but it actually just restores
> somewhere $,1rt(B I cannot figure out the behaviour.

> I understand
> that Cygwin has issues and needs this code, but couldn't it then be
> made to run only on Cygwin, such that on "proper" Unix systems, the
> config file is *not* updated every time I move the window before
> closing the application?

> > I understand that Cygwin has issues and needs this code, but
> > couldn't it then be made to run only on Cygwin, such that on
> > "proper" Unix systems, the config file is *not* updated every
> > time I move the window before closing the application?
> 
> What's the issue with updating the config file?

> > As I said earlier, this is really the job of a window manager,
> > *not* a configuration option. If your window manager cannot
> > restore the window to a size you control, then it's broken.
> > Let's not hack applications to work around that, please.
> 
> I disagree.  It's not a window manager's job to tell an
> application how big its window should be.

>  > As I said earlier, this is really the job
>  > of a window manager, *not* a configuration option. If your window
>  > manager cannot restore the window to a size you control, then it's
>  > broken. Let's not hack applications to work around that, please.
>
>  I disagree.  It's not a window manager's job to tell an application
>  how big its window should be.

> > Saving the geometry can cause several inconveniences, e.g. when
> > using a temporary dual screen setup, or sharing ~/.gitk between
> > multiple systems.  Additionally it can be argued that window
> > placement and sizing are the tasks of the window manager.  So
> > don't do that anymore.
> 
> I myself find it useful to have gitk remember the size and layout
> of the various panes, so I won't be applying this patch.
> 
> The saving/restoring of the window position was done by Mark
> Levedahl to make gitk work properly on Cygwin.  I agree it can be
> annoying on X and I would take a patch to inhibit the restoring of
> the window position when running under X.  I'd want an ack from
> Mark before making that change more generally.

> martin f krafft writes:
>
>  > May I suggest that such a patch actually disables writing of the
>  > config file, so that we won't get conflicts every time you sync
>  > config files via home-in-git?
>
>  Then various other bits of config won't be persistent, such as font
>  settings.  So I don't think that's a good idea.
>
>  It would be possible to make the config file be per-repository rather
>  than global, I guess, if that is something people want.  I personally
>  would find it annoying to have to change the font separately in each
>  repository, but tastes differ, I guess.
>
>  Paul.
>

> On Tue, Feb 26, 2008 at 2:35 AM, Mark Levedahl <mlevedahl@gmail.com> wrote:
> >  2) Perform a screen geometry check at load time to see if the corners
> >  of the main window would be visible before applying the main window
> >  geometry.
> 
> As a multi-screen-layouts, multi-machine user, this approach has my
> vote. I see a proto-patch from Edgar too. Even if the wm has similar
> capabilities, those will be limited to main window geometry, not
> inner-panel geometry so this is still useful.

> Gerrit Pape writes:
> 
>> Saving the geometry can cause several inconveniences, e.g. when using a
>> temporary dual screen setup, or sharing ~/.gitk between multiple
>> systems.  Additionally it can be argued that window placement and sizing
>> are the tasks of the window manager.  So don't do that anymore.
> 
> I myself find it useful to have gitk remember the size and layout of
> the various panes, so I won't be applying this patch.
> 
> The saving/restoring of the window position was done by Mark Levedahl
> to make gitk work properly on Cygwin.  I agree it can be annoying on X
> and I would take a patch to inhibit the restoring of the window
> position when running under X.  I'd want an ack from Mark before
> making that change more generally.