From: Alexander Gavrilov <angavrilov@gmail.com>
Date: Thu, 09 Oct 2008 08:12:04 GMT
Subject: Re: [PATCH 1/7] gitk: Enhance UI popup and accelerator handling.
Message-ID: <bb6f213e0810090112l60d7b4c3v42e42d9d769c6057@mail.gmail.com>
In-Reply-To: <18669.20487.88439.584567@cargo.ozlabs.ibm.com>

On Thu, Oct 9, 2008 at 4:27 AM, Paul Mackerras <paulus@samba.org> wrote:
>> +    catch { bind . <Shift-Key-XF86_Switch_VT_4> {newview 0} }
>
> What is this key, or why is this line needed?

It is a workaround for an apparent bug in interaction between Tk and
XKB on some systems.
XKB configuration files bind those symbols to Ctrl-Alt-F*
combinations, but Tk's keyboard
handiling code sees them when Shift-F* is pressed. I'll be glad to
find a better fix, but that's
all I could do for now.

The catch wrapper is there to avoid erroring out if that keysym does not exist.

>> @@ -2484,10 +2501,10 @@ proc keys {} {
>>
>> -[mc "<%s-Q>          Quit" $M1T]
>>  [mc "<Home>          Move to first commit"]
>>  [mc "<End>           Move to last commit"]
>>  [mc "<Up>, p, i      Move up one commit"]
>> @@ -2521,11 +2538,11 @@ proc keys {} {
>>  [mc "<%s-plus>       Increase font size" $M1T]
>>  [mc "<%s-KP->        Decrease font size" $M1T]
>>  [mc "<%s-minus>      Decrease font size" $M1T]
>> -[mc "<F5>            Update"]
>
> I think it is useful to have the accelerator keys listed in the key
> binding help.

I don't insist on this part of the patch, it just seemed to be redundant now.

>> @@ -7501,6 +7534,7 @@ proc domktag {} {
>>      addedtag $id
>>      dispneartags 0
>>      run refill_reflist
>> +    return 1
>>  }
>>
>>  proc redrawtags {id} {
>> @@ -7539,7 +7573,7 @@ proc mktagcan {} {
>>  }
>>
>>  proc mktaggo {} {
>> -    domktag
>> +    if {![domktag]} return
>
> You need to change the error returns in domktag to say "return 0",
> otherwise this will give a "can't use empty string as operand of "!""
> Tcl error.

I did not know this. I'm not that familiar with Tcl...

Alexander