[PATCH 0/3] gitk: macOS improvements

[PATCH 0/3] gitk: macOS improvements

[Carlo Marcelo Arenas Belón]

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

[PATCH 1/3] gitk: skip calling osascript to set frontmost for tk >= 8.6

[Carlo Marcelo Arenas Belón]

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 <carenas@gmail.com>
---
 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

[PATCH 2/3] gitk: avoid fatal error if `exec osascript` fails

[Carlo Marcelo Arenas Belón]

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 <carenas@gmail.com>
---
 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

[PATCH 3/3] gitk: check main window visibility before waiting for it to show

[Carlo Marcelo Arenas Belón]

From: Tobias Pietzsch <pietzsch@mycroft.speedport.ip>

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 <pietzsch@mycroft.speedport.ip>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 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

Re: [PATCH 3/3] gitk: check main window visibility before waiting for it to show

[Chris Torek]

On Tue, Aug 3, 2021 at 6:51 PM Carlo Marcelo Arenas Belón
<carenas@gmail.com> wrote:
>
> From: Tobias Pietzsch <pietzsch@mycroft.speedport.ip>
>
> 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 <pietzsch@mycroft.speedport.ip>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  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

Re: [PATCH 0/3] gitk: macOS improvements

[Carlo Arenas]

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
<carenas@gmail.com> 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