Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

SetKeyBinding with ModCtrl + rune #26

Closed
fenimore opened this issue May 30, 2017 · 13 comments
Closed

SetKeyBinding with ModCtrl + rune #26

fenimore opened this issue May 30, 2017 · 13 comments

Comments

@fenimore
Copy link
Contributor

fenimore commented May 30, 2017

Hi, is it already possible to add keybindings with the ModKeys?

If so I can't figure how from the docs and examples.

I see SetKeybinding takes (inter{}, func ).

How are ModKey events handled?

@marcusolsson
Copy link
Owner

There's currently no support for modifiers but this is definitely a must-have for an initial release. Apart from setting the API for SetKeyBinding there's not much work involved.

The reason it takes an interface{} is so that it can take special keys like KeySpace as well as letter keys.

Maybe we could just change the signature into SetKeybinding(r rune, k ModKey, func())?

Example usage:

ui.SetKeybinding('q', tui.ModNone, func() { ... }
ui.SetKeybinding(tui.KeySpace, tui.ModCtrl, func() { ... }

@fenimore
Copy link
Contributor Author

fenimore commented May 31, 2017

I noticed this enum in tcell

const (
	KeyCtrlSpace Key = iota
	KeyCtrlA
	KeyCtrlB
	KeyCtrlC
	KeyCtrlD
	KeyCtrlE
	KeyCtrlF
	KeyCtrlG
	KeyCtrlH
	KeyCtrlI
	KeyCtrlJ
	KeyCtrlK
	KeyCtrlL
	KeyCtrlM
	KeyCtrlN
	KeyCtrlO
	KeyCtrlP
	KeyCtrlQ
	KeyCtrlR
	KeyCtrlS
	KeyCtrlT
	KeyCtrlU
	KeyCtrlV
	KeyCtrlW
	KeyCtrlX
	KeyCtrlY
	KeyCtrlZ
	KeyCtrlLeftSq // Escape
	KeyCtrlBackslash
	KeyCtrlRightSq
	KeyCtrlCarat
	KeyCtrlUnderscore
)

So for example 'n' normally registers as 110 for the (Key or Ch, I forget), but when using the Ctrl modifier, it changes to 14 -- I didn't realize that using a modifier actually modifies the value. In any case, maybe we don't need to change the signature, but rather add the enum?

I forget why (I've been busy) but I didn't get it to work earlier

@fenimore
Copy link
Contributor Author

Ok... so this isn't an acceptable solution or anything, but I got the SetKeybinding() func to take as its first parameter an enum from the above, so tui.KeyCtrlN by (adding the enum)

func (b *Keybinding) Match(ev Event) bool {
	if b.Key == Key(ev.Ch) && ev.Key == Key(b.Ch) {
		return true
	}

	return ((b.Key == ev.Key) && (b.Ch == ev.Ch))
}

Problem is... I have no idea why this works. I mean, I have an idea, I logged the values of these when I smash the keyboard. But I don't really get where these values are coming from...

@marcusolsson
Copy link
Owner

marcusolsson commented Jun 1, 2017

Currently, Key is used for special keys like Space, Enter and arrow keys. The value of a Key doesn't really mean anything (other than the order it was defined). For example, in the const group you posted KeyCtrlSpace will be 0, KeyCtrlA will be 1, KeyCtrlB 2 and so on. Meanwhile, the value of Ch is the actual ASCII code of that letter. E.g.,

package main

import (
	"fmt"
)

func main() {
	fmt.Println(int('n')) // 110
}

While the ASCII code will be constant, the value of the Key might not be, and you can't really compare the two.

The "magic" is done here: https://github.com/marcusolsson/tui-go/blob/master/ui_tcell.go#L56-L61 where we do a type switch on the interface{}.

I'm actually in the middle of rewriting the event handling, so I'd probably hold off on this issue a bit. If you're interested, there's a branch called event-refactor. I'm actually leaning towards adopting the same idea as https://github.com/gdamore/tcell/blob/master/key.go#L45-L50 and use a ModMask type, hence my initial suggestion of introducing another parameter. Hopefully I'll get some time to work on this during the weekend.

@fenimore
Copy link
Contributor Author

fenimore commented Jun 1, 2017

Cool, thanks for clarifying -- I'll check in sometime next week 😸

@marcusolsson
Copy link
Owner

No worries! Thanks for your interest in tui-go!

@ghost
Copy link

ghost commented Jun 3, 2017

Good to know you are working on it, I'm working on this little thing https://github.com/gonzaloserrano/git-br and would love to bind shift or cmd + enter :-)

@marcusolsson
Copy link
Owner

Cool project @sp-gonzalo-serrano ! :)

While I am working on this, I'm currently trying to better understand terminal input handling. For example, from the tcell documentation:

Note that [control keys] overlap with other keys, perhaps.  For example, KeyCtrlH is the same as KeyBackspace.

Another example: a tcell key event could contain theoretically contain a ModCtrl along with the KeyCtrlSpace (duplicated Ctrl?).

I'm going to need to read up on this before attempting a solution. For now, I won't be able to make any promises as to when this will be done.

(Meanwhile, I've just submitted a related pull request for event handling, that will unfortunately break some existing code)

@marcusolsson
Copy link
Owner

So, there are currently two open pull requests (#28 and #29) with two different approaches to solving this. I'd love to hear your thoughts on what the pros and and cons are, and ultimately what you'd prefer.

@marcusolsson
Copy link
Owner

#28:

ui.SetKeybinding(tui.KeyUp, tui.ModShift | tui.ModAlt, func() { ... })
ui.SetKeybinding(tui.KeyCtrlSpace, tui.ModCtrl, func() { ... })

#29:

ui.SetKeybinding("Shift+Alt+Up", func() { ... })
ui.SetKeybinding("Ctrl+Space", func() { ... })

@fenimore
Copy link
Contributor Author

fenimore commented Jun 4, 2017

Hey! This all seems great, I really like the #29 method -- as you say, it might be more intuitive to new users who don't feel like understanding how the keybinding work 😉 -- also, my solution is more or less incomplete, (unable to perform the Shift-Enter needed by @sp-gonzalo-serrano )

It's not totally clear to me how the innerds of #29 work, but that also doesn't matter as long as I can Mod my keybindings 😄 . And the Emacs style sequences sounds awesome! I'm cool with closing #28 (I'm not invested in the pr I submitted) and this issue if your PR does the job.

Thanks for responding/taking care of this issue so timely!

@marcusolsson
Copy link
Owner

Thank you both for your feedback, especially @fenimore for both creating the issue and submitting the initial pull request!

Unfortunately, Shift+Enter and Ctrl+Enter does not yet seem to work for #29 either (although Alt+Enter does) so I'm guessing there's more to do there. Found this line that hints that Ctrl+Enter might have another code. Good thing is hopefully we'll be able to work on supporting the edge cases without users needing to bother.

I'm going ahead and merging #29, but it'll need more testing. Adding tests for more key sequences to keybinding_test.go will be much appreciated ;-)

@marcusolsson
Copy link
Owner

Also, created #30

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants