jroimartin / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
9.92k stars 608 forks source link

Keybinding prints key when switching to editable view. #65

Closed thekeymaker closed 8 years ago

thekeymaker commented 8 years ago

I am trying to write some code that edits a view with vim like key-bindings. The problem I am running into is the key I press is also getting inserted. I have put together some test code to hopefully explain better.

package main

import (
    "log"
    "github.com/jroimartin/gocui"
)

func edit(g *gocui.Gui, v *gocui.View) error {
    v.Editable = true
    return nil
}

func quit(g *gocui.Gui, v *gocui.View) error {
    return gocui.ErrQuit
}

func keybindings(g *gocui.Gui) error {
    if err := g.SetKeybinding("", gocui.KeyCtrlC, gocui.ModNone, quit); err != nil {return err}
    if err := g.SetKeybinding("main", 'i', gocui.ModNone, edit); err != nil {return err}

    return nil
}

func layout(g *gocui.Gui) error {
    maxX, maxY := g.Size()
    if v, err := g.SetView("main", 30, -1, maxX, maxY); err != nil {
        if err != gocui.ErrUnknownView {
            return err
        }

        v.Editable = false
        v.Wrap = true
        if _, err := g.SetCurrentView("main"); err != nil {return err}
    }
    return nil
}

func main() {
    g, _ := gocui.NewGui()
    defer g.Close()

    g.SetManagerFunc(layout)
    keybindings(g); 
    g.Cursor = true

    if err := g.MainLoop(); err != nil && err != gocui.ErrQuit {log.Panicln(err)}
}

What happens is when I press the 'i' key to change the main view from not-Editable to Editable it also inserts an i character. Is there a way to prevent this?

Thank you for any help!

jroimartin commented 8 years ago

Other people requested this behaviour before (https://github.com/jroimartin/gocui/issues/54#issuecomment-252038848) and after some tests I think it's good change. So, PR #66 should fix it, can you check?

Thanks! :)

jroimartin commented 8 years ago

BTW take into account that after this change if any keybinding matches the event, its handler is executed and the edition step is skipped. It means that, in your example, if you want to type i after making the view Editable, you will need to delete the keybinding.

if err := g.DeleteKeybinding("main", 'i', gocui.ModNone); err != nil {
        return err
}

Maybe this behaviour could be configurable if there is a rationale for that.

thekeymaker commented 8 years ago

I can defiantly try this out and get back to you later today. Thanks for the changes.

thekeymaker commented 8 years ago

That looks good for me. Your right, with this new commit I would have to assign the key again if I ever wanted that functionality back. Here is my previous code modified to do just that and works with your new commit:

package main

import (
    "log"
    "github.com/jroimartin/gocui"
)

func edit(g *gocui.Gui, v *gocui.View) error {
    v.Editable = true

    if err := g.DeleteKeybinding("main", 'i', gocui.ModNone); err != nil { return err }

    return nil
}

func escape_edit(g *gocui.Gui, v *gocui.View) error {
    v.Editable = false

    if err := g.SetKeybinding("main", 'i', gocui.ModNone, edit); err != nil {return err}

    return nil
}

func quit(g *gocui.Gui, v *gocui.View) error {
    return gocui.ErrQuit
}

func keybindings(g *gocui.Gui) error {
    if err := g.SetKeybinding("", gocui.KeyCtrlC, gocui.ModNone, quit); err != nil {return err}
    if err := g.SetKeybinding("main", 'i', gocui.ModNone, edit); err != nil {return err}
    if err := g.SetKeybinding("main", gocui.KeyEsc, gocui.ModNone, escape_edit); err != nil {return err}

    return nil
}

func layout(g *gocui.Gui) error {
    maxX, maxY := g.Size()
    if v, err := g.SetView("main", 30, -1, maxX, maxY); err != nil {
    if err != gocui.ErrUnknownView {
        return err
    }

    v.Editable = false
    v.Wrap = true
    if _, err := g.SetCurrentView("main"); err != nil {return err}
    }
    return nil
}

func main() {
    g, _ := gocui.NewGui()
    defer g.Close()

    g.SetManagerFunc(layout)
    keybindings(g); 
    g.Cursor = true
    g.InputEsc = true

    if err := g.MainLoop(); err != nil && err != gocui.ErrQuit {log.Panicln(err)}
}

I think this is an improvement because it always mean the key handler will capture that key unless the handler doesn't exist. Although, I do agree with your last comment that it may be nice to have the ability to change this functionality based on the situation.

Maybe another argument to the SetKeybinding() function to either capture or pass through the key press???

Don't know but like I said, I think this is an improvement. Thanks!

jroimartin commented 8 years ago

Let's keep it simple for now. At some point, if needed, we can add a Passthrough field to View.

BTW for your specific case, you can use the Editor interface to implement a custom edition mode. For instance, the following snippet shows how to create a simple vim editor:

https://gist.github.com/jroimartin/1ac98d3da7278fa18866c9cae0af6007

jroimartin commented 8 years ago

PR #67 is also related.

thekeymaker commented 8 years ago

Cool! I will have to take a look at that. Thanks for the help. It looks like you merge this change into master so you can close this issue if it isn't still in progress.

Cheers!