-
Notifications
You must be signed in to change notification settings - Fork 299
Conversation
A next step is to actually apply this to the special cases / hardcoded cases in |
There was a problem hiding this 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!!
There was a problem hiding this 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`: |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
browser/src/Editor/NeovimEditor.tsx
Outdated
if (key === "<f3>") { | ||
formatter.formatBuffer() | ||
|
||
if (inputManager.handleKey(key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so happy.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 {
?
There was a problem hiding this comment.
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(..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhhhh good point.
browser/src/Services/InputManager.ts
Outdated
public bind(keyChord: string, action: ActionOrCommand, filterFunction?: () => boolean) { | ||
// tslint:disable-line no-empty-block | ||
|
||
// TODO: add to existing binding |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
browser/src/Config.ts
Outdated
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") | ||
} | ||
} |
There was a problem hiding this comment.
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.
Need to incorporate |
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 bindingsOni.input.rebind
- binds a key to a command, clearing existing bindingsOni.input.unbind
- unbinds a keyOni.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 inconfig.js
, but I like the idea in #618 of having akeybindings.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
orunbind
.Remaining work: