[REGRESSION] git-gui

[REGRESSION] git-gui

[André Goddard Rosa]

Hi, all!
 
    After updating to git version 1.5.5.rc3 I always get the error below
when starting "git gui".
    Linux distributions used are opensuse 10 and opensuse Factory.
 
# git gui
Error in startup script: bad event type or keysym "["
    while executing
"bind $ui_comm <$M1B-Key-\[> {show_less_context;break}"
    file "/usr/bin/git-gui" linux 2718)

    I think it's related to git-gui 0.10 recently added into the tree.
 
Thanks a lot,
André

Re: [REGRESSION] git-gui

[Jonathan del Strother]

On Thu, Apr 3, 2008 at 2:43 PM, André Goddard Rosa <andre_rosa@lge.com> wrote:
> Hi, all!
>
>     After updating to git version 1.5.5.rc3 I always get the error below
>  when starting "git gui".
>     Linux distributions used are opensuse 10 and opensuse Factory.
>
>  # git gui
>  Error in startup script: bad event type or keysym "["
>     while executing
>  "bind $ui_comm <$M1B-Key-\[> {show_less_context;break}"
>     file "/usr/bin/git-gui" linux 2718)
>
>     I think it's related to git-gui 0.10 recently added into the tree.

Doh, I broke git :(

The problem is in c91ee2bd61 (Add shortcut keys for Show More/Less Context).
Works fine on my OS X 10.5 installation with tcl 8.4.7 - can anyone
suggest an alternative way of binding the left/right brackets?  I'm
not familiar with Tcl, and didn't manage to find a list of key names.

RES: [REGRESSION] git-gui

[André Goddard Rosa]

Hi,

>     I think it's related to git-gui 0.10 recently added into the tree.
> Doh, I broke git :(
> The problem is in c91ee2bd61 (Add shortcut keys for Show More/Less
Context).
> Works fine on my OS X 10.5 installation with tcl 8.4.7 - can anyone
suggest an alternative way of binding the left/right brackets?  I'm not
familiar with Tcl, and > 
> didn't manage to find a list of key names.

Yes, reverting this commit fix the issue.

Thank you,
André

Re: [REGRESSION] git-gui

[Michele Ballabio]

On Thursday 03 April 2008, Jonathan del Strother wrote:
> On Thu, Apr 3, 2008 at 2:43 PM, André Goddard Rosa <andre_rosa@lge.com> wrote:
> > Hi, all!
> >
> >     After updating to git version 1.5.5.rc3 I always get the error below
> >  when starting "git gui".
> >     Linux distributions used are opensuse 10 and opensuse Factory.
> >
> >  # git gui
> >  Error in startup script: bad event type or keysym "["
> >     while executing
> >  "bind $ui_comm <$M1B-Key-\[> {show_less_context;break}"
> >     file "/usr/bin/git-gui" linux 2718)
> >
> >     I think it's related to git-gui 0.10 recently added into the tree.
> 
> Doh, I broke git :(
> 
> The problem is in c91ee2bd61 (Add shortcut keys for Show More/Less Context).
> Works fine on my OS X 10.5 installation with tcl 8.4.7 - can anyone
> suggest an alternative way of binding the left/right brackets?  I'm
> not familiar with Tcl, and didn't manage to find a list of key names.

These changes should help (haven't tried):
	\[ -> bracketleft
	\] -> bracketright
but some European keyboards do not have easy access to brackets, so
other keys would be preferable (comma and period, for example, or 1 and 2).

See
	man keysyms
for a list of key names.

Re: [REGRESSION] git-gui

[Jonathan del Strother]

On Thu, Apr 3, 2008 at 10:04 PM, Michele Ballabio
<barra_cuda@katamail.com> wrote:
>
> On Thursday 03 April 2008, Jonathan del Strother wrote:
>  > On Thu, Apr 3, 2008 at 2:43 PM, André Goddard Rosa <andre_rosa@lge.com> wrote:
>  > > Hi, all!
>  > >
>  > >     After updating to git version 1.5.5.rc3 I always get the error below
>  > >  when starting "git gui".
>  > >     Linux distributions used are opensuse 10 and opensuse Factory.
>  > >
>  > >  # git gui
>  > >  Error in startup script: bad event type or keysym "["
>  > >     while executing
>  > >  "bind $ui_comm <$M1B-Key-\[> {show_less_context;break}"
>  > >     file "/usr/bin/git-gui" linux 2718)
>  > >
>  > >     I think it's related to git-gui 0.10 recently added into the tree.
>  >
>  > Doh, I broke git :(
>  >
>  > The problem is in c91ee2bd61 (Add shortcut keys for Show More/Less Context).
>  > Works fine on my OS X 10.5 installation with tcl 8.4.7 - can anyone
>  > suggest an alternative way of binding the left/right brackets?  I'm
>  > not familiar with Tcl, and didn't manage to find a list of key names.
>
>  These changes should help (haven't tried):
>         \[ -> bracketleft
>         \] -> bracketright
>  but some European keyboards do not have easy access to brackets, so
>  other keys would be preferable (comma and period, for example, or 1 and 2).
>
>  See
>         man keysyms
>  for a list of key names.
>

Anyone else got any opinions on an appropriate shortcut?  How about
'=' and '-' (+ and - without the shift), or are those not necessarily
together either?  (Are there really layouts where '[' and ']' aren't
next to each other?)

Re: [REGRESSION] git-gui

[Shawn O. Pearce]

Jonathan del Strother <maillist@steelskies.com> wrote:
> On Thu, Apr 3, 2008 at 10:04 PM, Michele Ballabio
> <barra_cuda@katamail.com> wrote:
> > On Thursday 03 April 2008, Jonathan del Strother wrote:
> >  > On Thu, Apr 3, 2008 at 2:43 PM, André Goddard Rosa <andre_rosa@lge.com> wrote:
> >  > >
> >  > >  # git gui
> >  > >  Error in startup script: bad event type or keysym "["
> >  > >     while executing
> >  > >  "bind $ui_comm <$M1B-Key-\[> {show_less_context;break}"
> >  >
> >  > Doh, I broke git :(

You and me both Jonathan.  I tested it on both Mac OS X and Win32,
and on Tcl/Tk 8.4.1, 8.4.10, 8.4.15, and 8.5.0.  Never ran into
this failure.  So the original poster must be running 8.4.0, and
8.4.0 must not support this binding.  Added in 8.4.1?  *sigh*

> >  These changes should help (haven't tried):
> >         \[ -> bracketleft
> >         \] -> bracketright
> >  but some European keyboards do not have easy access to brackets, so
> >  other keys would be preferable (comma and period, for example, or 1 and 2).
> 
> Anyone else got any opinions on an appropriate shortcut?  How about
> '=' and '-' (+ and - without the shift), or are those not necessarily
> together either?  (Are there really layouts where '[' and ']' aren't
> next to each other?)

I don't know, [ and ] seem reasonable to me.  Git is mostly a source
code control system.  A lot of languages use [ and ] as part of
their syntax.  If you are typing on a keyboard that is difficult to
access these keys on, you are probably already used to contorting
your fingers.  :-|

I wrote up a patch today based on Michele's suggested change.
It works everywhere I can test, but I don't have an 8.4.0
installation like the original poster.

Unless someone posts a patch to change the keys away from [ and ]
I say leave them as-is.  But I'm willing to entertain a change if
someone who cares writes a patch for it.

-- 
Shawn.

Re: [REGRESSION] git-gui

[Shawn O. Pearce]

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Jonathan del Strother <maillist@steelskies.com> wrote:
> > >  > >  # git gui
> > >  > >  Error in startup script: bad event type or keysym "["
> > >  > >     while executing
> > >  > >  "bind $ui_comm <$M1B-Key-\[> {show_less_context;break}"
> 
> > >  These changes should help (haven't tried):
> > >         \[ -> bracketleft
> > >         \] -> bracketright
> > >  but some European keyboards do not have easy access to brackets, so
> > >  other keys would be preferable (comma and period, for example, or 1 and 2).
> 
> I wrote up a patch today based on Michele's suggested change.

Hmm, maybe I should offer the patch.  :-)

--8>--
Subject: [PATCH] git-gui: Switch keybindings for [ and ] to bracketleft and bracketright

Thanks to Michele Ballabio for the quick fix.
This resolves the error introduced by c91ee2bd61.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 git-gui.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index d4e4c17..1a67105 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2739,8 +2739,8 @@ bind $ui_comm <$M1B-Key-v> {tk_textPaste %W; %W see insert; break}
 bind $ui_comm <$M1B-Key-V> {tk_textPaste %W; %W see insert; break}
 bind $ui_comm <$M1B-Key-a> {%W tag add sel 0.0 end;break}
 bind $ui_comm <$M1B-Key-A> {%W tag add sel 0.0 end;break}
-bind $ui_comm <$M1B-Key-\[> {show_less_context;break}
-bind $ui_comm <$M1B-Key-\]> {show_more_context;break}
+bind $ui_comm <$M1B-Key-bracketleft> {show_less_context;break}
+bind $ui_comm <$M1B-Key-bracketright> {show_more_context;break}
 
 bind $ui_diff <$M1B-Key-x> {tk_textCopy %W;break}
 bind $ui_diff <$M1B-Key-X> {tk_textCopy %W;break}
@@ -2784,8 +2784,8 @@ bind .   <$M1B-Key-t> do_add_selection
 bind .   <$M1B-Key-T> do_add_selection
 bind .   <$M1B-Key-i> do_add_all
 bind .   <$M1B-Key-I> do_add_all
-bind .   <$M1B-Key-\[> {show_less_context;break}
-bind .   <$M1B-Key-\]> {show_more_context;break}
+bind .   <$M1B-Key-bracketleft> {show_less_context;break}
+bind .   <$M1B-Key-bracketright> {show_more_context;break}
 bind .   <$M1B-Key-Return> do_commit
 foreach i [list $ui_index $ui_workdir] {
 	bind $i <Button-1>       "toggle_or_diff         $i %x %y; break"
-- 
1.5.5.rc3.148.gb77a7


-- 
Shawn.

Re: [REGRESSION] git-gui

[Marco Roeland]

On Thursday April 3rd 2008 at 21:33 Shawn O. Pearce wrote:

> Hmm, maybe I should offer the patch.  :-)
> 
> --8>--
> Subject: [PATCH] git-gui: Switch keybindings for [ and ] to bracketleft and bracketright
> 
> Thanks to Michele Ballabio for the quick fix.
> This resolves the error introduced by c91ee2bd61.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

This solves the problem on Debian GNU/Linux 'sid', with Tcl/Tk version
8.5.1. So somehow the Linux implementation seems to be the problem, not
the Tcl/Tk version as such. Man keysyms here does list "bracketleft" and
"bracketright" but no direct symbols for '[' or ']'. Thanks!
-- 
Marco Roeland

RES: [REGRESSION] git-gui

[André Goddard Rosa]

> Hmm, maybe I should offer the patch.  :-)
> 
> --8>--
> Subject: [PATCH] git-gui: Switch keybindings for [ and ] to 
> bracketleft and bracketright
> 
> Thanks to Michele Ballabio for the quick fix.
> This resolves the error introduced by c91ee2bd61.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

> This solves the problem on Debian GNU/Linux 'sid', with Tcl/Tk version
8.5.1. So somehow the Linux implementation seems to be the problem, not the
Tcl/Tk version as > such. Man keysyms here does list "bracketleft" and
"bracketright" but no direct symbols for '[' or ']'. Thanks!

It solves the issue indeed. Please apply.

Thank you all,
André

Re: [REGRESSION] git-gui

[Michele Ballabio]

On Friday 04 April 2008, Shawn O. Pearce wrote:
> > Anyone else got any opinions on an appropriate shortcut?  How about
> > '=' and '-' (+ and - without the shift), or are those not necessarily
> > together either?  (Are there really layouts where '[' and ']' aren't
> > next to each other?)
> 
> I don't know, [ and ] seem reasonable to me.  Git is mostly a source
> code control system.  A lot of languages use [ and ] as part of
> their syntax.  If you are typing on a keyboard that is difficult to
> access these keys on, you are probably already used to contorting
> your fingers.  :-|

To clarify, [ and ] are quite easy to type for me: the first problem
is that to type [ I have to press AltGr-è (or AltGr-8). With Ctrl, it
makes three keypresses: I can handle this, but - second problem - it
seems Tcl/Tk cannot (unless I'm wrong, which can also be very likely):

keys.tcl:
-->8--
#!/usr/bin/wish
bind . <Key> {
	puts "            %%K - %%k - %%N - %%T - %%D - %%A - %%d"
	puts "You pressed %K - %k - %N - %T - %D - %A - %d"
}
--8<--

Trying Ctrl+[:
$ ./keys.tcl 
            %K - %k - %N - %T - %D - %A - %d 
You pressed Control_R - 109 - 65508 - 2 - 109 - {} - ?? 
            %K - %k - %N - %T - %D - %A - %d 
You pressed ISO_Level3_Shift - 113 - 65027 - 2 - 113 - {} - ?? 
            %K - %k - %N - %T - %D - %A - %d 
You pressed ?? - 34 - 0 - 2 - 34 - ?

Trying [:
$ ./keys.tcl 
            %K - %k - %N - %T - %D - %A - %d 
You pressed ISO_Level3_Shift - 113 - 65027 - 2 - 113 - {} - ?? 
            %K - %k - %N - %T - %D - %A - %d 
You pressed ?? - 34 - 0 - 2 - 34 - [ - ?? 

So, it doesn't recognize [ at all at the keysym (%K) level.
No idea why. This is Tcl/Tk 8.4.15 on Linux.

> I wrote up a patch today based on Michele's suggested change.
> It works everywhere I can test, but I don't have an 8.4.0
> installation like the original poster.
> 
> Unless someone posts a patch to change the keys away from [ and ]
> I say leave them as-is.  But I'm willing to entertain a change if
> someone who cares writes a patch for it.

Here it is. Of course if I'm the only one that has problems with ]/[,
forget it. But it seems reasonable to me to do:

ctrl-+ and ctrl-=	-> more context
ctrl--			-> less context

Since these keybindings are used in gitk too, they should be safe. If
someone doesn't like this change, just say so. If someone thinks my
setup is strange and knows what to do to fix it up, let me know.

-->8--
From 4334c7690d80f7c05ec40f2efa4306ef88933469 Mon Sep 17 00:00:00 2001
From: Michele Ballabio <barra_cuda@katamail.com>
Date: Fri, 4 Apr 2008 18:29:48 +0200
Subject: [PATCH] git-gui: use +/- instead of ]/[ to show more/less context in diff

On some systems, brackets cannot be used as event details
(they don't have a keysym), so use +/- instead (both on
keyboard and keypad) and add ctrl-= as a synonym of ctrl-+
for convenience.

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 git-gui.sh |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 748efcc..744b51c 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2064,11 +2064,11 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} {
 
 	.mbar.commit add command -label [mc "Show Less Context"] \
 		-command show_less_context \
-		-accelerator $M1T-\[
+		-accelerator $M1T-\-
 
 	.mbar.commit add command -label [mc "Show More Context"] \
 		-command show_more_context \
-		-accelerator $M1T-\]
+		-accelerator "$M1T-+ $M1T-="
 
 	.mbar.commit add separator
 
@@ -2715,8 +2715,11 @@ bind $ui_comm <$M1B-Key-v> {tk_textPaste %W; %W see insert; break}
 bind $ui_comm <$M1B-Key-V> {tk_textPaste %W; %W see insert; break}
 bind $ui_comm <$M1B-Key-a> {%W tag add sel 0.0 end;break}
 bind $ui_comm <$M1B-Key-A> {%W tag add sel 0.0 end;break}
-bind $ui_comm <$M1B-Key-bracketleft> {show_less_context;break}
-bind $ui_comm <$M1B-Key-bracketright> {show_more_context;break}
+bind $ui_comm <$M1B-Key-minus> {show_less_context;break}
+bind $ui_comm <$M1B-Key-KP_Subtract> {show_less_context;break}
+bind $ui_comm <$M1B-Key-equal> {show_more_context;break}
+bind $ui_comm <$M1B-Key-plus> {show_more_context;break}
+bind $ui_comm <$M1B-Key-KP_Add> {show_more_context;break}
 
 bind $ui_diff <$M1B-Key-x> {tk_textCopy %W;break}
 bind $ui_diff <$M1B-Key-X> {tk_textCopy %W;break}
@@ -2760,8 +2763,11 @@ bind .   <$M1B-Key-t> do_add_selection
 bind .   <$M1B-Key-T> do_add_selection
 bind .   <$M1B-Key-i> do_add_all
 bind .   <$M1B-Key-I> do_add_all
-bind .   <$M1B-Key-bracketleft> {show_less_context;break}
-bind .   <$M1B-Key-bracketright> {show_more_context;break}
+bind .   <$M1B-Key-minus> {show_less_context;break}
+bind .   <$M1B-Key-KP_Subtract> {show_less_context;break}
+bind .   <$M1B-Key-equal> {show_more_context;break}
+bind .   <$M1B-Key-plus> {show_more_context;break}
+bind .   <$M1B-Key-KP_Add> {show_more_context;break}
 bind .   <$M1B-Key-Return> do_commit
 foreach i [list $ui_index $ui_workdir] {
 	bind $i <Button-1>       "toggle_or_diff         $i %x %y; break"
-- 
1.5.4.5

Re: [REGRESSION] git-gui

[Shawn O. Pearce]

Michele Ballabio <barra_cuda@katamail.com> wrote:
> 
> So, it doesn't recognize [ at all at the keysym (%K) level.
> No idea why. This is Tcl/Tk 8.4.15 on Linux.
...
> ctrl-+ and ctrl-=	-> more context
> ctrl--			-> less context

This is reasonable.  I like.
 
>  	.mbar.commit add command -label [mc "Show Less Context"] \
>  		-command show_less_context \
> -		-accelerator $M1T-\[
> +		-accelerator $M1T-\-
>  
>  	.mbar.commit add command -label [mc "Show More Context"] \
>  		-command show_more_context \
> -		-accelerator $M1T-\]
> +		-accelerator "$M1T-+ $M1T-="

Sadly Mac OS X does not like this accelerator in the menu.
It drops the accelerator entirely and just shows nothing.
I changed it to show "$M1T-=", but kept the bindings as
you had them below, so $M1T-+ is an alias.
  
> @@ -2715,8 +2715,11 @@ bind $ui_comm <$M1B-Key-v> {tk_textPaste %W; %W see insert; break}
>  bind $ui_comm <$M1B-Key-V> {tk_textPaste %W; %W see insert; break}
>  bind $ui_comm <$M1B-Key-a> {%W tag add sel 0.0 end;break}
>  bind $ui_comm <$M1B-Key-A> {%W tag add sel 0.0 end;break}
> -bind $ui_comm <$M1B-Key-bracketleft> {show_less_context;break}
> -bind $ui_comm <$M1B-Key-bracketright> {show_more_context;break}
> +bind $ui_comm <$M1B-Key-minus> {show_less_context;break}
> +bind $ui_comm <$M1B-Key-KP_Subtract> {show_less_context;break}
> +bind $ui_comm <$M1B-Key-equal> {show_more_context;break}
> +bind $ui_comm <$M1B-Key-plus> {show_more_context;break}
> +bind $ui_comm <$M1B-Key-KP_Add> {show_more_context;break}
>  
>  bind $ui_diff <$M1B-Key-x> {tk_textCopy %W;break}
>  bind $ui_diff <$M1B-Key-X> {tk_textCopy %W;break}
> @@ -2760,8 +2763,11 @@ bind .   <$M1B-Key-t> do_add_selection
>  bind .   <$M1B-Key-T> do_add_selection
>  bind .   <$M1B-Key-i> do_add_all
>  bind .   <$M1B-Key-I> do_add_all
> -bind .   <$M1B-Key-bracketleft> {show_less_context;break}
> -bind .   <$M1B-Key-bracketright> {show_more_context;break}
> +bind .   <$M1B-Key-minus> {show_less_context;break}
> +bind .   <$M1B-Key-KP_Subtract> {show_less_context;break}
> +bind .   <$M1B-Key-equal> {show_more_context;break}
> +bind .   <$M1B-Key-plus> {show_more_context;break}
> +bind .   <$M1B-Key-KP_Add> {show_more_context;break}
>  bind .   <$M1B-Key-Return> do_commit
>  foreach i [list $ui_index $ui_workdir] {
>  	bind $i <Button-1>       "toggle_or_diff         $i %x %y; break"
> -- 
> 1.5.4.5
> 

-- 
Shawn.

Re: [REGRESSION] git-gui

[David Kågedal]

"Jonathan del Strother" <maillist@steelskies.com> writes:

> Anyone else got any opinions on an appropriate shortcut?  How about
> '=' and '-' (+ and - without the shift), or are those not necessarily
> together either?  (Are there really layouts where '[' and ']' aren't
> next to each other?)

A Swedish keyboard has [] on alt-gr (Modeshift) 8 and 9.  Plus and
minus are unshifted and = is on shift-0.

So with a Swedish layout, it is annoying when zoom in is on = rather
than +, just because someone assumed that it would be harder to press
the more logical plus key.

-- 
David Kågedal

Re: [REGRESSION] git-gui

[Teemu Likonen]

David Kågedal kirjoitti (7.4.2008 klo 9.38):

> "Jonathan del Strother" <maillist@steelskies.com> writes:
> 
> > Anyone else got any opinions on an appropriate shortcut?  How about
> > '=' and '-' (+ and - without the shift), or are those not
> > necessarily together either?  (Are there really layouts where '['
> > and ']' aren't next to each other?)
> 
> A Swedish keyboard has [] on alt-gr (Modeshift) 8 and 9.  Plus and
> minus are unshifted and = is on shift-0.
> 
> So with a Swedish layout, it is annoying when zoom in is on = rather
> than +, just because someone assumed that it would be harder to press
> the more logical plus key.

The above also applies to Finnish and Norwegian keyboards, probably to
many more. The key pair '=' and '-' would be a poor UI decision. In
principle one should never assume that a key is in certain place in the
keyboard; it's much better idea to be logical with _characters_. So if
a key pair much be defined for different aspects of the same
functionality, let's choose logical pairs from common characters: +- []
{} () <> zZ aA bB ... (To me all these are OK.)

Re: [REGRESSION] git-gui

[Shawn O. Pearce]

Teemu Likonen <tlikonen@iki.fi> wrote:
> David Kågedal kirjoitti (7.4.2008 klo 9.38):
> > "Jonathan del Strother" <maillist@steelskies.com> writes:
> > 
> > > Anyone else got any opinions on an appropriate shortcut?  How about
> > > '=' and '-' (+ and - without the shift), or are those not
> > > necessarily together either?  (Are there really layouts where '['
> > > and ']' aren't next to each other?)
> > 
> > A Swedish keyboard has [] on alt-gr (Modeshift) 8 and 9.  Plus and
> > minus are unshifted and = is on shift-0.
> > 
> > So with a Swedish layout, it is annoying when zoom in is on = rather
> > than +, just because someone assumed that it would be harder to press
> > the more logical plus key.
> 
> The above also applies to Finnish and Norwegian keyboards, probably to
> many more. The key pair '=' and '-' would be a poor UI decision. In
> principle one should never assume that a key is in certain place in the
> keyboard; it's much better idea to be logical with _characters_. So if
> a key pair much be defined for different aspects of the same
> functionality, let's choose logical pairs from common characters: +- []
> {} () <> zZ aA bB ... (To me all these are OK.)

Well, Ctrl-+ and Ctrl-= are bound to the same action (increase
context), and Ctrl-- is bound to the opposite action (decrease
context).  So Swedish folks should be able to use Ctrl-+ and
be happy.

Except it doesn't show in the menubar as being a possible keyboard
driven action.  That was in the original patch, but I took it out
to enable the accelator to draw on Mac OS X.

-- 
Shawn.