Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

API: Input management #633

Merged
merged 37 commits into from
Sep 2, 2017
Merged

API: Input management #633

merged 37 commits into from
Sep 2, 2017

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Aug 19, 2017

FYI @cyansprite - These are some initial thoughts I have on an input bindings API.

This exposes the following APIs:

  • Oni.input.bind - binds a key to a command, preserving existing bindings
  • Oni.input.rebind - binds a key to a command, clearing existing bindings
  • Oni.input.unbind - unbinds a key
  • Oni.input.unbindAll - unbinds all keys (this is meant to be called internally, after re-loading the config)

The idea is I could use bind a few different ways:

Simplest cases

Map key -> command

Oni.input.bind("<f12>", "oni.editor.gotoDefinition")

Map key -> function

Oni.input.bind("<f3>", () => console.log("I pressed F3"))

In addition, there is an optional 'enabled' argument. This can be used to determine if the key binding should be processed.

Filtering

In either case, whether we are binding to a command or a function, we can specify a filtering function as the third parameter. If it returns true, the keybinding is enabled:

Oni.input.bind("<f12>", "oni.editor.gotoDefinition", () => Oni.editors.activeEditor.mode === "normal")

It's possible that there could be multiple mappings, like:
Oni.input.bind("<f12>", "oni.editor.gotoDefinition", () => Oni.editors.activeEditor.mode === "normal")
Oni.input.bind("<f12>", () => console.log("hello"), () => Oni.editors.activeEditor.mode === "insert")

In this case, when the user presses <f12>, we would look at the items they bound in order, and the first one that passes the filter function will get executed.

If there is no keybinding handled for a key, we'll pass it on to the active editor for that to handle (ie, hand off to Neovim).

I'd also like to add some helpers so that these can be a bit more concise. Something like:
Oni.input.bind("<f12>", "oni.editor.gotoDefinition", () => Oni.editors.activeEditor.mode === "normal") => Oni.input.bind("<f12>", "oni.editor.gotoDefinition", Oni.helpers.isNormalMode)

Customization

Initially, we can put these in the activate method in config.js, but I like the idea in #618 of having a keybindings.js/keybindings.json.

As mentioned above, prior to re-loading the config, we'll need to clear all bindings. We'll also apply default key bindings, which the user can then rebind or unbind.

Remaining work:

  • Add arrow keys as default for menu navigation

@bryphe
Copy link
Member Author

bryphe commented Aug 19, 2017

A next step is to actually apply this to the special cases / hardcoded cases in NeovimEditor.tsx. I think that will be a good proof of concept whether this is actually flexible enough and usable.

Copy link
Contributor

@cyansprite cyansprite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great will look and do a more indepth review when I get to a computer!!

Copy link
Contributor

@cyansprite cyansprite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking awesome!

PLAN.md Outdated

- Add option to hide commands in screen

- Create these 'commands' in `Commands.ts`:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will look into this, I have a few things I want to implement with the menu that isn't supported such as selecting multiple files (c-i i.e. tab, (increment)) and opening things silently (i.e. open a file but leave the menu where it's open (whatever + shift so, c-S to split horizontal silent)), and to use c-w to delete word back and c-u to delete entire line of input field (linux standards I love)

The bindings will hook up to quickOpen which will be nice, I'm really liking this commit!

PLAN.md Outdated
quickOpen.select.openSplitHorizontal
quickOpen.select.openSplitVertical

completion.complete
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a way to hook in register complete when I get to it. I haven't looked much into auto completion yet though.
I'm thinking the register name, followed by the first line
extra lines will show in what autocomplete uses for "documentation help" right now.

if (key === "<f3>") {
formatter.formatBuffer()

if (inputManager.handleKey(key)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so happy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too... it's satisfying to see this chunk of special cases go away!

@@ -66,6 +67,10 @@ export class Oni extends EventEmitter implements Oni.Plugin.Api {
return editorManager
}

public get input(): Oni.InputManager {
return inputManager
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you change this to

public get inputManager(): Oni.InputManager {

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is basically the API definition, I was thinking that:
Oni.input.bind(..)
is more readable than:
Oni.inputManager.bind(..)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhhhh good point.

public bind(keyChord: string, action: ActionOrCommand, filterFunction?: () => boolean) {
// tslint:disable-line no-empty-block

// TODO: add to existing binding
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean add to existing binding?
Are you saying if I bind c-j to next menu item also except c-n? perhaps have two methods, bind to override and bindMore to add them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, basically if I do something like:

const isVisualMode = () => Oni.editors.activeEditor.mode === "visual"
const isNormalMode = () => Oni.editors.activeEditor.mode === "normal"
Oni.input.bind("<C-c>", "someCommand",  isVisualMode)
Oni.input.bind("<C-c>", "someOtherCommand", isNormalMode)

Only the last binding would work right now, since we clear out old bindings. That's the difference I had between bind and rebind - bind preserves existing bindings, but rebind clears them all.

Perhaps it's better just to have bind and unbind, and if it ends up being a common pattern to call unbind + bind, we can bring rebind back as a convenience.

oni.input.bind("<M-c>", "editor.clipboard.yank", () => oni.editors.activeEditor.mode === "visual")
oni.input.bind("<M-v>", "editor.clipboard.paste", () => oni.editors.activeEditor.mode === "insert")
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this to

const prefix = Platform.isLinux() || Platform.isWindows() ? "C" : "M"
if (this.getValue("editor.clipboard.enabled")) {
    oni.input.bind("<"+prefix+"-c>", "editor.clipboard.yank", () => oni.editors.activeEditor.mode === "visual")
    oni.input.bind("<"+prefix+"-v>", "editor.clipboard.paste", () => oni.editors.activeEditor.mode === "insert")
}

this can also serve as example for users who use both mac and windows, if they want one config file they can use this syntax so they don't have to have different config files.

@bryphe
Copy link
Member Author

bryphe commented Aug 25, 2017

Need to incorporate cmdline_normal change here

@bryphe bryphe merged commit 6d953f0 into master Sep 2, 2017
@bryphe bryphe deleted the extr0py/api/input branch July 4, 2018 05:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants