Esc will quit when not in popup, fixes #197

[ghost]

No description provided.

[codecov-io]

Codecov Report

Merging #200 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #200   +/-   ##
=======================================
  Coverage   73.32%   73.32%           
=======================================
  Files           8        8           
  Lines        1286     1286           
=======================================
  Hits          943      943           
  Misses        342      342           
  Partials        1        1

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 042c833...110ff38. Read the comment docs.

[jesseduffield]

Could you also add "main": we can escape from main when we're resolving merge conflicts. Also we should rename this function to 'escape' now that it's generally just a function for when you either want to escape the current view, or escape the app.

I'd prefer a solution where globally set keybindings are ignored if there is a matching keybinding for the current view; that way we wouldn't need to add to this list for every new view that is escapable.

What are your thoughts of patching the gocui fork like so. It restricts keybindings to only be matched with one handler at a time, with preference given to matching on views over matching globally.

diff --git a/vendor/github.com/jesseduffield/gocui/gui.go b/vendor/github.com/jesseduffield/gocui/gui.go
index 28a52d1..b746ca0 100644
--- a/vendor/github.com/jesseduffield/gocui/gui.go
+++ b/vendor/github.com/jesseduffield/gocui/gui.go
@@ -653,17 +653,29 @@ func (g *Gui) onKey(ev *termbox.Event) error {
 // execKeybindings executes the keybinding handlers that match the passed view
 // and event. The value of matched is true if there is a match and no errors.
 func (g *Gui) execKeybindings(v *View, ev *termbox.Event) (matched bool, err error) {
-       matched = false
+       var globalKb *keybinding
        for _, kb := range g.keybindings {
                if kb.handler == nil {
                        continue
                }
-               if kb.matchKeypress(Key(ev.Key), ev.Ch, Modifier(ev.Mod)) && kb.matchView(v) {
+               if !kb.matchKeypress(Key(ev.Key), ev.Ch, Modifier(ev.Mod)) {
+                       continue
+               }
+               if kb.matchView(v) {
                        if err := kb.handler(g, v); err != nil {
                                return false, err
                        }
-                       matched = true
+                       return true, nil
+               }
+               if kb.viewName == "" {
+                       globalKb = kb
+               }
+       }
+       if globalKb != nil {
+               if err := globalKb.handler(g, v); err != nil {
+                       return false, err
                }
+               return true, nil
        }
-       return matched, nil
+       return false, nil
 }
diff --git a/vendor/github.com/jesseduffield/gocui/keybinding.go b/vendor/github.com/jesseduffield/gocui/keybinding.go
index 7efdb75..82d1acc 100644
--- a/vendor/github.com/jesseduffield/gocui/keybinding.go
+++ b/vendor/github.com/jesseduffield/gocui/keybinding.go
@@ -38,9 +38,6 @@ func (kb *keybinding) matchView(v *View) bool {
        if v.Editable == true && kb.ch != 0 {
                return false
        }
-       if kb.viewName == "" {
-               return true
-       }
        return v != nil && kb.viewName == v.name
 }

[ghost]

Cool works perfectly, commit that to your fork and I'll update the dep.

[jesseduffield]

I've just committed to the fork so you're good to go :)

Actually if we're going down this path, I'd request to change the function name back to 'quit' because now if the handler is ever called it will indeed quit the application.

[jesseduffield]

@remyabel is this ready for re-review?

[ghost]

nope, looks like I accidentally committed some unrelated code.

[ghost]

Should be good to go now.

[jesseduffield]

awesome :)