# [PATCH 0/3] gitk: macOS improvements 6 messages from 2021-08-04 to 2021-08-04. Participants: Carlo Marcelo Arenas Belón, Carlo Arenas, Chris Torek. Thread: https://gitlist.dev/t/56210 ## Carlo Marcelo Arenas Belón, 2021-08-04 01:08 Subject: [PATCH 0/3] gitk: macOS improvements Message-ID: <20210804010900.33133-1-carenas@gmail.com> URL: https://gitlist.dev/e/20210804010900.33133-1-carenas%40gmail.com ``` The following patches improve the user experience for gitk in macOS by avoiding to abort if the terminal where it is running hasn't been authorized for Automation as reported a few[1,2] times already. It has been tested in macOS 11.5.1 using both the system tk (8.5) and the latest (8.6.11) from brew, the third[3] patch is needed for using 8.6 and has been included with the gitk version from brew as well. Carlo Marcelo Arenas Belón (2): gitk: skip calling osascript to set frontmost for tk >= 8.6 gitk: avoid fatal error if `exec osascript` fails Tobias Pietzsch (1): gitk: check main window visibility before waiting for it to show gitk | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) [1] https://lore.kernel.org/git/20180724065120.7664-1-sunshine@sunshineco.com/ [2] https://lore.kernel.org/git/20201025175149.11853-1-dev+git@drbeat.li/ [3] https://lore.kernel.org/git/pull.944.git.git.1610234771966.gitgitgadget@gmail.com/ -- 2.33.0.rc0.433.g9a510e7e11 ``` ## Carlo Marcelo Arenas Belón, 2021-08-04 01:08 Subject: [PATCH 1/3] gitk: skip calling osascript to set frontmost for tk >= 8.6 Message-ID: <20210804010900.33133-2-carenas@gmail.com> URL: https://gitlist.dev/e/20210804010900.33133-2-carenas%40gmail.com In-Reply-To: <20210804010900.33133-1-carenas@gmail.com> ``` 76bf6ff (gitk: On OSX, bring the gitk window to front, 2013-04-24) adds an osascript to set the gitk window to the front to workaround and unfortunate default in older tk versions to add it to the back. tk 8.6 or newer do not need that workaround so add a conditional check to skip it and while at it update the comment with the new name for the OS. Signed-off-by: Carlo Marcelo Arenas Belón --- gitk | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/gitk b/gitk index 23d9dd1..f68d983 100755 --- a/gitk +++ b/gitk @@ -12288,13 +12288,15 @@ if {[catch {package require Tk 8.4} err]} { exit 1 } -# on OSX bring the current Wish process window to front +# on macOS bring the current Wish process window to front if needed if {[tk windowingsystem] eq "aqua"} { - exec osascript -e [format { - tell application "System Events" - set frontmost of processes whose unix id is %d to true - end tell - } [pid] ] + if {$tcl_version < 8.6} { + exec osascript -e [format { + tell application "System Events" + set frontmost of processes whose unix id is %d to true + end tell + } [pid] ] + } } # Unset GIT_TRACE var if set -- 2.33.0.rc0.433.g9a510e7e11 ``` ## Carlo Marcelo Arenas Belón, 2021-08-04 01:09 Subject: [PATCH 3/3] gitk: check main window visibility before waiting for it to show Message-ID: <20210804010900.33133-4-carenas@gmail.com> URL: https://gitlist.dev/e/20210804010900.33133-4-carenas%40gmail.com In-Reply-To: <20210804010900.33133-1-carenas@gmail.com> ``` From: Tobias Pietzsch If the main window is already visible when gitk waits for it to become visible, gitk hangs forever. This commit adds a check whether the window is already visible. See https://wiki.tcl-lang.org/page/tkwait+visibility Signed-off-by: Tobias Pietzsch Signed-off-by: Carlo Marcelo Arenas Belón --- gitk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitk b/gitk index b62c12f..cc587b5 100755 --- a/gitk +++ b/gitk @@ -12664,7 +12664,7 @@ catch { wm iconphoto . -default gitlogo gitlogo32 } # wait for the window to become visible -tkwait visibility . +if {![winfo viewable .]} {tkwait visibility .} set_window_title update readrefs -- 2.33.0.rc0.433.g9a510e7e11 ``` ## Carlo Marcelo Arenas Belón, 2021-08-04 01:08 Subject: [PATCH 2/3] gitk: avoid fatal error if `exec osascript` fails Message-ID: <20210804010900.33133-3-carenas@gmail.com> URL: https://gitlist.dev/e/20210804010900.33133-3-carenas%40gmail.com In-Reply-To: <20210804010900.33133-1-carenas@gmail.com> ``` starting with macOS 10.14 calling "System Events" requires an "Automation" grant, that will result in a fatal error showing: Error in startup script: 66:111: execution error: Not authorized to send Apple events to System Events. (-1743) instead of aborting catch the error and print a suitable warning. Signed-off-by: Carlo Marcelo Arenas Belón --- gitk | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/gitk b/gitk index f68d983..b62c12f 100755 --- a/gitk +++ b/gitk @@ -12291,11 +12291,15 @@ if {[catch {package require Tk 8.4} err]} { # on macOS bring the current Wish process window to front if needed if {[tk windowingsystem] eq "aqua"} { if {$tcl_version < 8.6} { - exec osascript -e [format { - tell application "System Events" - set frontmost of processes whose unix id is %d to true - end tell - } [pid] ] + if {[catch { + exec osascript -e [format { + tell application "System Events" + set frontmost of processes whose unix id is %d to true + end tell + } [pid] ] + } ]} { + puts stderr [mc "Warning: 'System Events' access denied"] + } } } -- 2.33.0.rc0.433.g9a510e7e11 ``` ## Carlo Arenas, 2021-08-04 02:37 Subject: Re: [PATCH 0/3] gitk: macOS improvements Message-ID: URL: https://gitlist.dev/e/CAPUEspj56Pc6Ubbj%2BnY2gM3f7TX3PfvnSj8BGcMcG2evaeG4vw%40mail.gmail.com In-Reply-To: <20210804010900.33133-1-carenas@gmail.com> ``` Tested in Windows (with 8.6) as well not to introduce any regressions +CC Tobias (gmail instead of the one bouncing in the SoB) On Tue, Aug 3, 2021 at 6:09 PM Carlo Marcelo Arenas Belón wrote: > > The following patches improve the user experience for gitk in macOS > by avoiding to abort if the terminal where it is running hasn't been > authorized for Automation as reported a few[1,2] times already. > > It has been tested in macOS 11.5.1 using both the system tk (8.5) and > the latest (8.6.11) from brew, the third[3] patch is needed for using > 8.6 and has been included with the gitk version from brew as well. > > Carlo Marcelo Arenas Belón (2): > gitk: skip calling osascript to set frontmost for tk >= 8.6 > gitk: avoid fatal error if `exec osascript` fails > > Tobias Pietzsch (1): > gitk: check main window visibility before waiting for it to show > > gitk | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > [1] https://lore.kernel.org/git/20180724065120.7664-1-sunshine@sunshineco.com/ > [2] https://lore.kernel.org/git/20201025175149.11853-1-dev+git@drbeat.li/ > [3] https://lore.kernel.org/git/pull.944.git.git.1610234771966.gitgitgadget@gmail.com/ > -- > 2.33.0.rc0.433.g9a510e7e11 ``` ## Chris Torek, 2021-08-04 06:27 Subject: Re: [PATCH 3/3] gitk: check main window visibility before waiting for it to show Message-ID: URL: https://gitlist.dev/e/CAPx1Gvdp85H_EY9qLY3jkxktXCDAwJyznMWfDDGHy56db7sc6Q%40mail.gmail.com In-Reply-To: <20210804010900.33133-4-carenas@gmail.com> ``` On Tue, Aug 3, 2021 at 6:51 PM Carlo Marcelo Arenas Belón wrote: > > From: Tobias Pietzsch > > If the main window is already visible when gitk waits for it to > become visible, gitk hangs forever. > This commit adds a check whether the window is already visible. > See https://wiki.tcl-lang.org/page/tkwait+visibility > > Signed-off-by: Tobias Pietzsch > Signed-off-by: Carlo Marcelo Arenas Belón > --- > gitk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gitk b/gitk > index b62c12f..cc587b5 100755 > --- a/gitk > +++ b/gitk > @@ -12664,7 +12664,7 @@ catch { > wm iconphoto . -default gitlogo gitlogo32 > } > # wait for the window to become visible > -tkwait visibility . > +if {![winfo viewable .]} {tkwait visibility .} > set_window_title > update > readrefs I have no objection to this patch (and the other two look fine to me) but I would like to say that this whole idea looks racy in general, and that it would be better to fix this inside Tk itself: the internal visibility-wait should be doing the visibility-check already. Given that it's obviously *not*, this, as a workaround, seems OK. Chris ```