Re: [PATCH] git-gui: Work around freeze problem with dialogs in Mac OS X

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 22, 2010, 19:01 UTC
Message-ID: <7vocbpsivd.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1jp8k4n.1lz3bce9u857kM%lists@haller-berlin.de>

lists@haller-berlin.de (Stefan Haller) writes:

Show 10 quoted lines

> Tk 8.5 on Mac OS X has a bug whereby a dialog opened from a key
> binding will hang; see issue 3044863 in the Tk issue tracker.
> <http://sourceforge.net/tracker/?func=detail&aid=3044863&group_id=12997&atid=112997>
>
> To work around this, we perform commands that open a dialog after
> a brief delay; 150 ms seems to be a good compromise between short
> enough as to be not annoying, and long enough to reliably work
> around the issue.
>
> Signed-off-by: Stefan Haller <stefan@haller-berlin.de>

Is 150ms applicable no matter how fast or slow your Mac is, or is Mac so monoculture that everybody's machine has more or less the same performance characteristics? IOW does this need to be autoadjusted?

I see a lot of wrapping around foo::dialog; without knowing much about Tcl, I wonder if it would be simpler, less error prone and more future proof to add the wrapping logic around something commonly used from them, e.g. class::make_dialog.

Show 88 quoted lines

> ---
> I already sent this two days ago, but it didn't seem to appear on the
> list for some reason, so I'm resending it. Apologies if you see this
> twice.
>
>  git-gui.sh |   39 +++++++++++++++++++++++++++------------
>  1 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 4617f29..394c2a0 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -3560,6 +3560,21 @@ if {[info exists repo_config(gui.wmstate)]} {
>     catch {wm state . $repo_config(gui.wmstate)}
>  }
>  
> +proc mac_freeze_workaround {cmd} {
> +   if {[is_MacOSX] && $::have_tk85} {
> +       # Tk 8.5 on Mac OS X has a bug whereby a dialog opened from a key
> +       # binding will hang; see issue 3044863 in the Tk issue tracker.
> +       # <http://sourceforge.net/tracker/?func=detail&aid=3044863&group_id=12997&atid=112997>
> +       #
> +       # To work around this, we perform commands that open a dialog after a brief
> +       # delay; 150 ms seems to be a good compromise between short enough as to be
> +       # not annoying, and long enough to reliably work around the issue.
> +       after 150 $cmd
> +   } else {
> +       $cmd
> +   }
> +}
> +
>  # -- Key Bindings
>  #
>  bind $ui_comm <$M1B-Key-Return> {do_commit;break}
> @@ -3567,8 +3582,8 @@ bind $ui_comm <$M1B-Key-t> {do_add_selection;break}
>  bind $ui_comm <$M1B-Key-T> {do_add_selection;break}
>  bind $ui_comm <$M1B-Key-u> {do_unstage_selection;break}
>  bind $ui_comm <$M1B-Key-U> {do_unstage_selection;break}
> -bind $ui_comm <$M1B-Key-j> {do_revert_selection;break}
> -bind $ui_comm <$M1B-Key-J> {do_revert_selection;break}
> +bind $ui_comm <$M1B-Key-j> {mac_freeze_workaround do_revert_selection;break}
> +bind $ui_comm <$M1B-Key-J> {mac_freeze_workaround do_revert_selection;break}
>  bind $ui_comm <$M1B-Key-i> {do_add_all;break}
>  bind $ui_comm <$M1B-Key-I> {do_add_all;break}
>  bind $ui_comm <$M1B-Key-x> {tk_textCut %W;break}
> @@ -3606,16 +3621,16 @@ bind $ui_diff <Control-Key-f> {catch {%W yview scroll  1 pages};break}
>  bind $ui_diff <Button-1>   {focus %W}
>  
>  if {[is_enabled branch]} {
> -   bind . <$M1B-Key-n> branch_create::dialog
> -   bind . <$M1B-Key-N> branch_create::dialog
> -   bind . <$M1B-Key-o> branch_checkout::dialog
> -   bind . <$M1B-Key-O> branch_checkout::dialog
> -   bind . <$M1B-Key-m> merge::dialog
> -   bind . <$M1B-Key-M> merge::dialog
> +   bind . <$M1B-Key-n> {mac_freeze_workaround branch_create::dialog}
> +   bind . <$M1B-Key-N> {mac_freeze_workaround branch_create::dialog}
> +   bind . <$M1B-Key-o> {mac_freeze_workaround branch_checkout::dialog}
> +   bind . <$M1B-Key-O> {mac_freeze_workaround branch_checkout::dialog}
> +   bind . <$M1B-Key-m> {mac_freeze_workaround merge::dialog}
> +   bind . <$M1B-Key-M> {mac_freeze_workaround merge::dialog}
>  }
>  if {[is_enabled transport]} {
> -   bind . <$M1B-Key-p> do_push_anywhere
> -   bind . <$M1B-Key-P> do_push_anywhere
> +   bind . <$M1B-Key-p> {mac_freeze_workaround do_push_anywhere}
> +   bind . <$M1B-Key-P> {mac_freeze_workaround do_push_anywhere}
>  }
>  
>  bind .   <Key-F5>     ui_do_rescan
> @@ -3625,8 +3640,8 @@ bind .   <$M1B-Key-s> do_signoff
>  bind .   <$M1B-Key-S> do_signoff
>  bind .   <$M1B-Key-t> do_add_selection
>  bind .   <$M1B-Key-T> do_add_selection
> -bind .   <$M1B-Key-j> do_revert_selection
> -bind .   <$M1B-Key-J> do_revert_selection
> +bind .   <$M1B-Key-j> {mac_freeze_workaround do_revert_selection}
> +bind .   <$M1B-Key-J> {mac_freeze_workaround do_revert_selection}
>  bind .   <$M1B-Key-i> do_add_all
>  bind .   <$M1B-Key-I> do_add_all
>  bind .   <$M1B-Key-minus> {show_less_context;break}
> -- 
> 1.7.3.4.g200b9
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html