Skip to contentSkip to conversation

gitk: Fix transient windows on Win32 and MacOS.

PATCH Applied Alexander Gavrilov 3 participants 7 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 gitk: Fix transient windows on Win32 and MacOS.

Transient windows cause problems on these platforms:

- On Win32 the windows appear in the top left corner of the screen. In order to fix it, this patch causes them to be explicitly centered on their parents by an idle handler.

- On MacOS with Tk 8.4 they appear without a title bar. Since it is clearly unacceptable, this patch disables transient on that platform.

---

NOTE: This patch DEPRECATES the earlier one, called
"Explicitly position popup windows"

Alexander

 gitk |   44 +++++++++++++++++++++++++++++++-------------
 1 files changed, 31 insertions(+), 13 deletions(-)
gitk +31 -13
index 9b2a6e5..e6aafe8 100755
@@ -1739,6 +1739,24 @@ proc removehead {id name} {
     unset headids($name)
 }
 
+proc make_transient {window origin} {
+    global have_tk85
+
+    # In MacOS Tk 8.4 transient appears to work by setting
+    # overrideredirect, which is utterly useless, since the
+    # windows get no border, and are not even kept above
+    # the parent.
+    if {!$have_tk85 && [tk windowingsystem] eq {aqua}} return
+
+    wm transient $window $origin
+
+    # Windows fails to place transient windows normally, so
+    # schedule a callback to center them on the parent.
+    if {[tk windowingsystem] eq {win32}} {
+	after idle [list tk::PlaceWindow $window widget $origin]
+    }
+}
+
 proc show_error {w top msg} {
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
@@ -1754,7 +1772,7 @@ proc show_error {w top msg} {
 proc error_popup {msg {owner .}} {
     set w .error
     toplevel $w
-    wm transient $w $owner
+    make_transient $w $owner
     show_error $w $w $msg
 }
 
@@ -1763,7 +1781,7 @@ proc confirm_popup {msg {owner .}} {
     set confirm_ok 0
     set w .confirm
     toplevel $w
-    wm transient $w $owner
+    make_transient $w $owner
     message $w.m -text $msg -justify center -aspect 400
     pack $w.m -side top -fill x -padx 20 -pady 20
     button $w.ok -text [mc OK] -command "set confirm_ok 1; destroy $w"
@@ -2558,7 +2576,7 @@ proc about {} {
     }
     toplevel $w
     wm title $w [mc "About gitk"]
-    wm transient $w .
+    make_transient $w .
     message $w.m -text [mc "
 Gitk - a commit viewer for git
 
@@ -2587,7 +2605,7 @@ proc keys {} {
     }
     toplevel $w
     wm title $w [mc "Gitk key bindings"]
-    wm transient $w .
+    make_transient $w .
     message $w.m -text "
 [mc "Gitk key bindings:"]
 
@@ -3669,7 +3687,7 @@ proc vieweditor {top n title} {
 
     toplevel $top
     wm title $top $title
-    wm transient $top .
+    make_transient $top .
 
     # View name
     frame $top.nfr
@@ -7912,7 +7930,7 @@ proc mkpatch {} {
     set patchtop $top
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Generate patch"]
     grid $top.title - -pady 10
     label $top.from -text [mc "From:"]
@@ -7999,7 +8017,7 @@ proc mktag {} {
     set mktagtop $top
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Create tag"]
     grid $top.title - -pady 10
     label $top.id -text [mc "ID:"]
@@ -8102,7 +8120,7 @@ proc writecommit {} {
     set wrcomtop $top
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Write commit to file"]
     grid $top.title - -pady 10
     label $top.id -text [mc "ID:"]
@@ -8159,7 +8177,7 @@ proc mkbranch {} {
     set top .makebranch
     catch {destroy $top}
     toplevel $top
-    wm transient $top .
+    make_transient $top .
     label $top.title -text [mc "Create new branch"]
     grid $top.title - -pady 10
     label $top.id -text [mc "ID:"]
@@ -8322,7 +8340,7 @@ proc resethead {} {
     set confirm_ok 0
     set w ".confirmreset"
     toplevel $w
-    wm transient $w .
+    make_transient $w .
     wm title $w [mc "Confirm reset"]
     message $w.m -text \
 	[mc "Reset branch %s to %s?" $mainhead [string range $rowmenuid 0 7]] \
@@ -8502,7 +8520,7 @@ proc showrefs {} {
     }
     toplevel $top
     wm title $top [mc "Tags and heads: %s" [file tail [pwd]]]
-    wm transient $top .
+    make_transient $top .
     text $top.list -background $bgcolor -foreground $fgcolor \
 	-selectbackground $selectbgcolor -font mainfont \
 	-xscrollcommand "$top.xsb set" -yscrollcommand "$top.ysb set" \
@@ -9844,7 +9862,7 @@ proc choosefont {font which} {
 	font create sample
 	eval font config sample [font actual $font]
 	toplevel $top
-	wm transient $top $prefstop
+	make_transient $top $prefstop
 	wm title $top [mc "Gitk font chooser"]
 	label $top.l -textvariable fontparam(which)
 	pack $top.l -side top
@@ -9961,7 +9979,7 @@ proc doprefs {} {
     }
     toplevel $top
     wm title $top [mc "Gitk preferences"]
-    wm transient $top .
+    make_transient $top .
     label $top.ldisp -text [mc "Commit list display options"]
     grid $top.ldisp - -sticky w -pady 10
     label $top.spacer -text " "
Signature · 1 line
1.6.0.3.15.gb8d36
#1 lore raw
Quoted 7 lines · Alexander Gavrilov

Transient windows cause problems on these platforms:

- On Win32 the windows appear in the top left corner of the screen. In order to fix it, this patch causes them to be explicitly centered on their parents by an idle handler.

Quoted 5 lines

- On MacOS with Tk 8.4 they appear without a title bar. Since it is clearly unacceptable, this patch disables transient on that platform.

Thanks, applied.
Paul.

#2 lore raw
In reply to Alexander Gavrilov
Quoted 1 line · Alexander Gavrilov

Transient windows cause problems on these platforms:

...

gitk

Quoted 2 lines
gitk +0 -0
index 9b2a6e5..e6aafe8 100755

I'd appreciate if you could make it a habit to base your patches on versions of gitk etc. that are available from a public repository so that the index information is useful:

Applying: gitk: Fix transient windows on Win32 and MacOS.
warning: gitk-git/gitk has type 100644, expected 100755
error: patch failed: gitk-git/gitk:1754
error: gitk-git/gitk: patch does not apply
fatal: sha1 information is lacking or useless (gitk-git/gitk). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge.
Patch failed at 0001.

I'm stopping here... :-(

-- Hannes

#3 lore raw
Quoted 8 lines · Johannes Sixt
Quoted 1 line · Alexander Gavrilov

Transient windows cause problems on these platforms:

...

gitk

Quoted 2 lines
gitk +0 -0
index 9b2a6e5..e6aafe8 100755

I'd appreciate if you could make it a habit to base your patches on versions of gitk etc. that are available from a public repository

You mean, like, git://git.kernel.org/pub/scm/gitk/gitk.git, for instance? :) That is the primary repository for gitk and it seems to be what Alexander bases his patches on.

Paul.

#4 lore raw
Quoted 4 lines · Paul Mackerras
Quoted 3 lines · Johannes Sixt
Quoted 1 line · Alexander Gavrilov

Transient windows cause problems on these platforms:

...

Quoted 8 lines
Quoted 4 lines

gitk

Quoted 2 lines
gitk +0 -0
index 9b2a6e5..e6aafe8 100755

I'd appreciate if you could make it a habit to base your patches on versions of gitk etc. that are available from a public repository

You mean, like, git://git.kernel.org/pub/scm/gitk/gitk.git, for instance? :) That is the primary repository for gitk and it seems to be what Alexander bases his patches on.

Yes, I mean exactly this repository. It doesn't have 9b2a6e5:

$ git fetch gitk
$ git show 9b2a6e5 --
fatal: bad revision '9b2a6e5'

I assume that Alexander has another patch applied in addition to the one that he submitted, which, therefore, is no longer "based on a publically available version".

-- Hannes

#5 lore raw
Quoted 3 lines · Johannes Sixt

I assume that Alexander has another patch applied in addition to the one that he submitted, which, therefore, is no longer "based on a publically available version".

I'm sorry, it is indeed applied over another patch (attached because I only have access to Gmail Web UI right now). These patches eventually come from two ends of one long series that has been gradually applied over the time, so I still think of them as a unit.

Alexander

From fc6b3728e15483734ee44a35d5023660a7da8882 Mon Sep 17 00:00:00 2001
From: Alexander Gavrilov <[email protected]>
Date: Sun, 9 Nov 2008 13:00:45 +0300
Subject: [PATCH] gitk: Add accelerators to frequently used menu commands.

This commit documents keyboard accelerators used for menu commands in the menu, as it is usually done, and adds some more, e.g. F4 to invoke Edit View.

The changes include a workaround for handling Shift-F4 on systems where XKB binds special XF86_Switch_VT_* symbols to Ctrl-Alt-F* combinations. Tk often receives these codes when Shift-F* is pressed, so it is necessary to bind the relevant actions to them as well.

---
 gitk |   36 +++++++++++++++++++++++++++++-------
 1 files changed, 29 insertions(+), 7 deletions(-)
gitk +29 -7
index 18d153e..9b2a6e5 100755
@@ -1801,6 +1801,11 @@ proc setoptions {} {
 # command to invoke for command, or {variable value} for radiobutton
 proc makemenu {m items} {
     menu $m
+    if {[tk windowingsystem] eq {aqua}} {
+	set Meta1 Cmd
+    } else {
+	set Meta1 Ctrl
+    }
     foreach i $items {
 	set name [mc [lindex $i 1]]
 	set type [lindex $i 2]
@@ -1826,7 +1831,9 @@ proc makemenu {m items} {
 		    -value [lindex $thing 1]
 	    }
 	}
-	eval $m add $params [lrange $i 4 end]
+	set tail [lrange $i 4 end]
+	regsub -all {\yMeta1\y} $tail $Meta1 tail
+	eval $m add $params $tail
 	if {$type eq "cascade"} {
 	    makemenu $m.$submenu $thing
 	}
@@ -1860,17 +1867,17 @@ proc makewindow {} {
     makemenu .bar {
 	{mc "File" cascade {
 	    {mc "Update" command updatecommits -accelerator F5}
-	    {mc "Reload" command reloadcommits}
+	    {mc "Reload" command reloadcommits -accelerator Meta1-F5}
 	    {mc "Reread references" command rereadrefs}
-	    {mc "List references" command showrefs}
-	    {mc "Quit" command doquit}
+	    {mc "List references" command showrefs -accelerator F2}
+	    {mc "Quit" command doquit -accelerator Meta1-Q}
 	}}
 	{mc "Edit" cascade {
 	    {mc "Preferences" command doprefs}
 	}}
 	{mc "View" cascade {
-	    {mc "New view..." command {newview 0}}
-	    {mc "Edit view..." command editview -state disabled}
+	    {mc "New view..." command {newview 0} -accelerator Shift-F4}
+	    {mc "Edit view..." command editview -state disabled -accelerator F4}
 	    {mc "Delete view" command delview -state disabled}
 	    {xx "" separator}
 	    {mc "All files" radiobutton {selectedview 0} -command {showview 0}}
@@ -2232,7 +2239,12 @@ proc makewindow {} {
     bindkey <Key-Return> {dofind 1 1}
     bindkey ? {dofind -1 1}
     bindkey f nextfile
-    bindkey <F5> updatecommits
+    bind . <F5> updatecommits
+    bind . <$M1B-F5> reloadcommits
+    bind . <F2> showrefs
+    bind . <Shift-F4> {newview 0}
+    catch { bind . <Shift-Key-XF86_Switch_VT_4> {newview 0} }
+    bind . <F4> edit_or_newview
     bind . <$M1B-q> doquit
     bind . <$M1B-f> {dofind 1 1}
     bind . <$M1B-g> {dofind 1 0}
@@ -3624,6 +3636,16 @@ proc decode_view_opts {n view_args} {
     set newviewopts($n,args) [shellarglist $oargs]
 }
 
+proc edit_or_newview {} {
+    global curview
+
+    if {$curview > 0} {
+	editview
+    } else {
+	newview 0
+    }
+}
+
 proc editview {} {
     global curview
     global viewname viewperm newviewname newviewopts
Signature · 1 line
1.6.0.2.GIT
#6 lore raw
Quoted 4 lines · Alexander Gavrilov

I'm sorry, it is indeed applied over another patch (attached because I only have access to Gmail Web UI right now). These patches eventually come from two ends of one long series that has been gradually applied over the time, so I still think of them as a unit.

I applied the attached patch.

Thanks,
Paul.