[BUG] Parse() doesn't parse Alt, Shift modifiers and converts Ctrl to a key

[abitrolly]

Parse() fails process Alt+ and Shift+ combinations. See the code below.

package main

import (
	"github.com/awesome-gocui/gocui"
	"github.com/kr/pretty"
)

func main() {
	pretty.Println(gocui.Parse("q"))
	pretty.Println(gocui.Parse("ctrl+q"))
	pretty.Println(gocui.Parse("shift+q"))
	pretty.Println(gocui.Parse("alt+q"))
}

This outputs,

int32(113) gocui.Modifier(0) nil
gocui.Key(17) gocui.Modifier(0) nil
nil gocui.Modifier(0) &errors.errorString{s:"no such keybind"}
nil gocui.Modifier(0) &errors.errorString{s:"no such keybind"}

Another strange behavior is that https://pkg.go.dev/github.com/awesome-gocui/gocui#Parse should return modifier separately from Key, but it for Ctrl+ there is no Modifier and it is mixed into the key.

To Reproduce

https://play.golang.org/p/UPWMipkgA9U

Expected behavior

Key('q') Modifier() nil
Key('q') Modifier('ctrl') nil
Key('q') Modifier('shift') nil
Key('q') Modifier('alt') nil

[KoditkarVedant]

Hi @mjarkk @glvr182, I was looking to contribute to this project. I'm new to Go lang but I do have experience in other language. So I was going thought this bug description. I wonder is this really a bug because looking at the code in Parse method it is behaving as expected.

[mjarkk]

@KoditkarVedant You are correct the codes doesn't seem to support this tough i think you expect this kind of input to work.

You are welcome to add support for this!

[KoditkarVedant]

@mjarkk What are your views on using the https://github.com/awesome-gocui/keybinding instead of maintaining the separate implementation in this repo.

[mjarkk]

I think we should not use it to keep the amount of dependencies low.

[abitrolly]

@mjarkk how many new dependencies does it add?

[dankox]

I'm not really sure what is the purpose of the keybinding library. It doesn't provide any meaningful functionality when used alone and it is not a CLI (which I thought it is) as it only provides functions to translate into gocui keys which can be used only by gocui (which looks kinda similar to original gocui parsing functions with some additions).

I think that parsing of keybinds should be done in gocui as it's used by it so it would make more sense.
Regarding keybinding library, that could be maybe changed (or rather newly created) into CLI which would prompt for keypresses and some comments and create a Go file with all the function calls for the specific keys (probably some template function).

Anyway, just my two cents :) Regarding the original question, I'm with @mjarkk on this... there is no need to add another dependency on a file which was extracted from this library and put into separate library just to add it back to this library by a another dependency.
Also, I'm not sure if it's even possible, because that library is dependent on this library, creating circular dependency ;) I know it's a problem in one project between packages, but not sure how Golang would act when it's between modules.

[abitrolly]

For better portability across different frameworks, it would be nice to have some reference keybinding lib. Having it in separate repo at least should make it clear what approach is used, which constants are provided, how keys are referenced from code and from an application config.

A demo that allows to test different keys and combination would be useful. Especially for different language layouts.

[dankox]

I see, for such a case maybe the keybinding library makes sense. Although I guess it could be created as a submodule in this repo instead so the parser is only in one place, or maybe the parser in keybinding should use the parser from this library. Or users could just use this library to parse the keys because this library will be downloaded anyway.

Nevertheless, you can't really make dependency in this library on keybinding, because it is a circular dependency and keybinding uses gocui for the keys to produce them.

[abitrolly]

Ideally both should be merged into https://pkg.go.dev/golang.org/x/term instead of adding another layer to the cake. At least key codes and handling logic for them could be universal.

[dankox]

Hmmm... I guess we are talking about two different things here. keybinding or gocui keybinds are not based on golang.org/x/term library.
The keys used here are obtained from underlaying tcell library, which handles consoles/terminals across multiple platforms and have its own specific way of dealing with it.

golang.org/x/term doesn't really fit in at all, as it only provides support for classic shell type terminal with prompt and have no special key handling available to user from what I remember (only readline or something like that). It doesn't even have key codes publicly available, so using it as a third-party library to translate those keys into gocui can't be made even if one would want to.

On the other hand tcell is terminal UI library which provides full control of a terminal it runs on with cell based view and all kinds of other stuff.

So these are really totally different libraries and that idea is not applicable here at all. But this is probably going already a bit off-topic. If you have a specific use case in mind which would bring some light to the problem you see, I think it might be good to open a new issue and describe it there exactly with specific examples.