From: Johannes Sixt Date: Wed, 03 Dec 2025 10:15:20 GMT Subject: Re: [PATCH] gitk: fix history window panes position Message-ID: <9a9441d5-fb64-4718-8765-852e66458598@kdbg.org> In-Reply-To: Am 02.12.25 um 14:08 schrieb tobias.boesch@miele.com: > From: Tobias Boesch > Date: Thu, 27 Nov 2025 11:27:16 +0100 > Subject: [PATCH] gitk: fix history window panes position > > When the history window panes in are resized > by moving either of the two sashes and then > the gitk window is vertically resized, > the sashes fall back into their previous > position without respecting the users wish > for resizing. You say "the Gitk windows is resized vertically". Did you mean "resized horizontally"? If I change only the height of the Gitk window, the widths of the top panel (history, author, date) aren't changed at all. However, if I change only the width, the symptoms do occur. Also, the error is not limited to the upper half of the window. The lower panel (patch text, file list) also forgets the last used position when the window size is changed. Can we fix this, too? > Save the sash position when the sashes are > moved to make them keep their position when > the window is resized afterwards. > > When the gitk window is opened and maximized > on a screen, then closed and opened on a > screen smaller than the previously used one, > the author pane and time pane of the history > window only are a few pixels wide and their > contents are barely visible. > Widen the two panes on start of gitk to a > reasonable fixed size that shows a good amount > of text of authors and time. I did this test: 0. Make Gitk significantly less than the screen size. 1. Adjust pane size to 1/2 history, 1/3 author, 1/6 date. 2. Maximize window via "Maximize" button. 3. Restore window via "Maximize" button. After 2., the pane widths are scaled with the window width and retain their proportions (or so it seems). But after 3., the pane widths are completely scrambled. The date pane is far too wide (wider than in the maximized window), the history pane steals a lot of the remaining width, and the author pane is squished to a minimal size. The behavior without the patch was better in this regard, because the proportions of the memoized panel widths were retained. > > Signed-off-by: Tobias Boesch > --- > > Notes: > Debug print statements are left in the code for easier > testing by maintainers. > They will be removed when the review is finished. > > gitk-git/gitk | 41 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/gitk-git/gitk b/gitk-git/gitk > index 7f62c8041d..6fbc2588fb 100755 > --- a/gitk-git/gitk > +++ b/gitk-git/gitk > @@ -2471,6 +2471,23 @@ proc makewindow {} { > -xscrollincr $linespc \ > -yscrollincr $linespc -yscrollcommand "scrollcanv $cscroll" > .tf.histframe.pwclist add $canv > + bind .tf.histframe.pwclist.canv { > + global oldsash > + set parent [regsub {\.[A-Za-z]+$} %W ""] > + puts "Canvas (pwclist) configuration changed saving sash \ > + position if parent panedwindow $parent is initialised \ > + (oldsash exist)" > + if {[info exists oldsash($parent)]} { > + set s0 [$parent sashpos 0] > + set s1 [$parent sashpos 1] > + puts " Sash0 is $s0" > + puts " Sash1 is $s1" > + set oldsash($parent) [list $s0 $s1] > + puts " oldsash saved for $parent" > + } else { > + puts " oldsash not yet existing so oldsash is not saved for $parent" > + } > + } I wonder why this handler is only installed for one of the three panes. Does panedwindow not have any features that can notify us to store the current sash positions? Can we perhaps bind to its or instead? At any rate, a callback like this is large enough (even without debugging code) to be moved to its own function. Is there a reason that proc resizeclistpanes cannot be reused in some way? > set canv2 .tf.histframe.pwclist.canv2 > canvas $canv2 \ > -selectbackground $selectbgcolor \ > @@ -3116,30 +3133,53 @@ proc savestuff {w} { > > proc resizeclistpanes {win w} { > global oldwidth oldsash > + puts "Starting resizeclistpanes..." > if {[info exists oldwidth($win)]} { > if {[info exists oldsash($win)]} { > + puts " Using oldsash from window" > set s0 [lindex $oldsash($win) 0] > set s1 [lindex $oldsash($win) 1] > + puts " Sash0 is $s0" > + puts " Sash1 is $s1" > } else { > + puts " New window creation detected" > + puts " Width is $w" > + puts " Using sash from window sashpos directly" > set s0 [$win sashpos 0] > set s1 [$win sashpos 1] > + puts " Sash0 is $s0" > + puts " Sash1 is $s1" > + if {$s1 > $w - 140} { > + puts " Sash1 greater than width - 140, setting max size" > + set s1 [expr {$w - 140}] > + if {$s0 > $s1 - 300} {> + puts " Sash0 greater than sash1 - 300, setting max size" > + set s0 [expr {$s1 - 300}] So, these aren't "max size", but actually "minimal width". It is strange that the minimal width of the author pane is only corrected if the date pane is too small as well. I have an issue with this. If the user makes the panes small, the wider versions are forced on them. The user's wish should have priority. I understand that you want to restore the widths to a sane size after the "maximize-restore" operation has caused the degenerated widths. But doesn't this solution just paper over the real bug that the window resize operation doesn't heed the pane width proportions? > + } > + } > } > if {$w < 60} { > + puts " Narrow window ($w), scaling sash in dependency to window width" > set sash0 [expr {int($w/2 - 2)}] > set sash1 [expr {int($w*5/6 - 2)}] > } else { > + puts " Wide window ($w), scaling sash in dependency to old width, oldsash and window width" > set factor [expr {1.0 * $w / $oldwidth($win)}] > set sash0 [expr {int($factor * [lindex $s0 0])}] > set sash1 [expr {int($factor * [lindex $s1 0])}] Not a problem of this patch, but I wonder why we have [lindex] here. > if {$sash0 < 30} { > + puts " Sash0 too small, setting min size" > set sash0 30 > } > if {$sash1 < $sash0 + 20} { > + puts " Sash1 smaller than sash0 + 20, setting min size" > set sash1 [expr {$sash0 + 20}] > } > if {$sash1 > $w - 10} { > + puts " Sash1 greater than width - 140, setting max size" > set sash1 [expr {$w - 10}] > if {$sash0 > $sash1 - 20} { > + puts " Sash0 greater than sash1 - 300, setting max size" > set sash0 [expr {$sash1 - 20}] > } > } > @@ -3149,6 +3189,7 @@ proc resizeclistpanes {win w} { > set oldsash($win) [list $sash0 $sash1] > } > set oldwidth($win) $w > + puts "Finished resizeclistpanes..." > } > > proc resizecdetpanes {win w} { -- Hannes