{"thread":{"id":"56210","subject":"[PATCH 0/3] gitk: macOS improvements","startedAt":"2021-08-04T01:09:28Z","lastAt":"2021-08-04T06:27:34Z","messageCount":6,"participants":["Carlo Marcelo Arenas Belón","Carlo Arenas","Chris Torek"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"431916","messageId":"20210804010900.33133-1-carenas@gmail.com","threadId":"56210","inReplyTo":null,"subject":"[PATCH 0/3] gitk: macOS improvements","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-08-04T01:08:57Z","receivedAt":"2021-08-04T01:09:28Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"The following patches improve the user experience for gitk in macOS\nby avoiding to abort if the terminal where it is running hasn't been\nauthorized for Automation as reported a few[1,2] times already.\n\nIt has been tested in macOS 11.5.1 using both the system tk (8.5) and\nthe latest (8.6.11) from brew, the third[3] patch is needed for using\n8.6 and has been included with the gitk version from brew as well.\n\nCarlo Marcelo Arenas Belón (2):\n gitk: skip calling osascript to set frontmost for tk >= 8.6\n gitk: avoid fatal error if `exec osascript` fails\n\nTobias Pietzsch (1):\n gitk: check main window visibility before waiting for it to show\n\n gitk | 20 +++++++++++++-------\n 1 file changed, 13 insertions(+), 7 deletions(-)\n\n[1] https://lore.kernel.org/git/20180724065120.7664-1-sunshine@sunshineco.com/\n[2] https://lore.kernel.org/git/20201025175149.11853-1-dev+git@drbeat.li/\n[3] https://lore.kernel.org/git/pull.944.git.git.1610234771966.gitgitgadget@gmail.com/\n-- \n2.33.0.rc0.433.g9a510e7e11\n"},{"id":"431917","messageId":"20210804010900.33133-2-carenas@gmail.com","threadId":"56210","inReplyTo":"20210804010900.33133-1-carenas@gmail.com","subject":"[PATCH 1/3] gitk: skip calling osascript to set frontmost for tk >= 8.6","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-08-04T01:08:58Z","receivedAt":"2021-08-04T01:09:29Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"76bf6ff (gitk: On OSX, bring the gitk window to front, 2013-04-24) adds\nan osascript to set the gitk window to the front to workaround and\nunfortunate default in older tk versions to add it to the back.\n\ntk 8.6 or newer do not need that workaround so add a conditional check\nto skip it and while at it update the comment with the new name for the\nOS.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n gitk | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 23d9dd1..f68d983 100755\n--- a/gitk\n+++ b/gitk\n@@ -12288,13 +12288,15 @@ if {[catch {package require Tk 8.4} err]} {\n exit 1\n }\n \n-# on OSX bring the current Wish process window to front\n+# on macOS bring the current Wish process window to front if needed\n if {[tk windowingsystem] eq \"aqua\"} {\n- exec osascript -e [format {\n- tell application \"System Events\"\n- set frontmost of processes whose unix id is %d to true\n- end tell\n- } [pid] ]\n+ if {$tcl_version < 8.6} {\n+ exec osascript -e [format {\n+ tell application \"System Events\"\n+ set frontmost of processes whose unix id is %d to true\n+ end tell\n+ } [pid] ]\n+ }\n }\n \n # Unset GIT_TRACE var if set\n-- \n2.33.0.rc0.433.g9a510e7e11\n\n"},{"id":"431918","messageId":"20210804010900.33133-4-carenas@gmail.com","threadId":"56210","inReplyTo":"20210804010900.33133-1-carenas@gmail.com","subject":"[PATCH 3/3] gitk: check main window visibility before waiting for it to show","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-08-04T01:09:00Z","receivedAt":"2021-08-04T01:09:31Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"From: Tobias Pietzsch \n\nIf the main window is already visible when gitk waits for it to\nbecome visible, gitk hangs forever.\nThis commit adds a check whether the window is already visible.\nSee https://wiki.tcl-lang.org/page/tkwait+visibility\n\nSigned-off-by: Tobias Pietzsch \nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n gitk | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitk b/gitk\nindex b62c12f..cc587b5 100755\n--- a/gitk\n+++ b/gitk\n@@ -12664,7 +12664,7 @@ catch {\n wm iconphoto . -default gitlogo gitlogo32\n }\n # wait for the window to become visible\n-tkwait visibility .\n+if {![winfo viewable .]} {tkwait visibility .}\n set_window_title\n update\n readrefs\n-- \n2.33.0.rc0.433.g9a510e7e11\n\n"},{"id":"431919","messageId":"20210804010900.33133-3-carenas@gmail.com","threadId":"56210","inReplyTo":"20210804010900.33133-1-carenas@gmail.com","subject":"[PATCH 2/3] gitk: avoid fatal error if `exec osascript` fails","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-08-04T01:08:59Z","receivedAt":"2021-08-04T01:09:32Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"starting with macOS 10.14 calling \"System Events\" requires an\n\"Automation\" grant, that will result in a fatal error showing:\n\n Error in startup script: 66:111: execution error: Not authorized to send Apple events to System Events. (-1743)\n\ninstead of aborting catch the error and print a suitable warning.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n gitk | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex f68d983..b62c12f 100755\n--- a/gitk\n+++ b/gitk\n@@ -12291,11 +12291,15 @@ if {[catch {package require Tk 8.4} err]} {\n # on macOS bring the current Wish process window to front if needed\n if {[tk windowingsystem] eq \"aqua\"} {\n if {$tcl_version < 8.6} {\n- exec osascript -e [format {\n- tell application \"System Events\"\n- set frontmost of processes whose unix id is %d to true\n- end tell\n- } [pid] ]\n+ if {[catch {\n+ exec osascript -e [format {\n+ tell application \"System Events\"\n+ set frontmost of processes whose unix id is %d to true\n+ end tell\n+ } [pid] ]\n+ } ]} {\n+ puts stderr [mc \"Warning: 'System Events' access denied\"]\n+ }\n }\n }\n \n-- \n2.33.0.rc0.433.g9a510e7e11\n\n"},{"id":"431923","messageId":"CAPUEspj56Pc6Ubbj+nY2gM3f7TX3PfvnSj8BGcMcG2evaeG4vw@mail.gmail.com","threadId":"56210","inReplyTo":"20210804010900.33133-1-carenas@gmail.com","subject":"Re: [PATCH 0/3] gitk: macOS improvements","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-08-04T02:37:11Z","receivedAt":"2021-08-04T02:37:30Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Tested in Windows (with 8.6) as well not to introduce any regressions\n\n+CC Tobias (gmail instead of the one bouncing in the SoB)\n\nOn Tue, Aug 3, 2021 at 6:09 PM Carlo Marcelo Arenas Belón\n wrote:\n>\n> The following patches improve the user experience for gitk in macOS\n> by avoiding to abort if the terminal where it is running hasn't been\n> authorized for Automation as reported a few[1,2] times already.\n>\n> It has been tested in macOS 11.5.1 using both the system tk (8.5) and\n> the latest (8.6.11) from brew, the third[3] patch is needed for using\n> 8.6 and has been included with the gitk version from brew as well.\n>\n> Carlo Marcelo Arenas Belón (2):\n> gitk: skip calling osascript to set frontmost for tk >= 8.6\n> gitk: avoid fatal error if `exec osascript` fails\n>\n> Tobias Pietzsch (1):\n> gitk: check main window visibility before waiting for it to show\n>\n> gitk | 20 +++++++++++++-------\n> 1 file changed, 13 insertions(+), 7 deletions(-)\n>\n> [1] https://lore.kernel.org/git/20180724065120.7664-1-sunshine@sunshineco.com/\n> [2] https://lore.kernel.org/git/20201025175149.11853-1-dev+git@drbeat.li/\n> [3] https://lore.kernel.org/git/pull.944.git.git.1610234771966.gitgitgadget@gmail.com/\n> --\n> 2.33.0.rc0.433.g9a510e7e11\n"},{"id":"431939","messageId":"CAPx1Gvdp85H_EY9qLY3jkxktXCDAwJyznMWfDDGHy56db7sc6Q@mail.gmail.com","threadId":"56210","inReplyTo":"20210804010900.33133-4-carenas@gmail.com","subject":"Re: [PATCH 3/3] gitk: check main window visibility before waiting for it to show","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2021-08-04T06:27:14Z","receivedAt":"2021-08-04T06:27:34Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Tue, Aug 3, 2021 at 6:51 PM Carlo Marcelo Arenas Belón\n wrote:\n>\n> From: Tobias Pietzsch \n>\n> If the main window is already visible when gitk waits for it to\n> become visible, gitk hangs forever.\n> This commit adds a check whether the window is already visible.\n> See https://wiki.tcl-lang.org/page/tkwait+visibility\n>\n> Signed-off-by: Tobias Pietzsch \n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n> gitk | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/gitk b/gitk\n> index b62c12f..cc587b5 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -12664,7 +12664,7 @@ catch {\n> wm iconphoto . -default gitlogo gitlogo32\n> }\n> # wait for the window to become visible\n> -tkwait visibility .\n> +if {![winfo viewable .]} {tkwait visibility .}\n> set_window_title\n> update\n> readrefs\n\nI have no objection to this patch (and the other two look fine to me)\nbut I would like to say that this whole idea looks racy in general,\nand that it would be better to fix this inside Tk itself: the internal\nvisibility-wait should be doing the visibility-check already. Given\nthat it's obviously *not*, this, as a workaround, seems OK.\n\nChris\n"}]}