Skip to content

Commit

Permalink
fix(clipboard/#3353): Command-v should paste in normal mode (#3361)
Browse files Browse the repository at this point in the history
__Issue:__ Command-v should paste in normal mode, since it doesn't conflict with a Vim binding, and is the intuitive behavior.

__Defect:__ The `"editor.clipboard.pasteAction"` the keybinding was bound to only ran in insert/command line modes.

__Fix:__ 
- Allow the command to run in select, normal, and visual modes
- Ensure we have the latest state by bringing in the current vim context
- Move bindings to `Feature_Clipboard`

Fixes #3353
  • Loading branch information
bryphe authored Apr 2, 2021
1 parent 572a512 commit b12edb3
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- #3346 - Extensions: Fix parse errors for progress and updateConfigurationOption APIs (related #3321)
- #3326 - Lifecycle: Delay process termination until cleanup actions have run (fixes #3270, thanks @timbertson !)
- #3359 - SCM: Don't show diffs for untracked or ignored files (fixes #3355)
- #3361 - Clipboard: Add command+v binding for paste in normal mode (fixes #3353)

### Performance

Expand Down
32 changes: 32 additions & 0 deletions src/Feature/Clipboard/Feature_Clipboard.re
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,38 @@ module Commands = {
);
};

module Keybindings = {
open Feature_Input.Schema;
let pasteNonMac =
bind(
~key="<C-V>",
~command=Commands.paste.id,
// The WhenExpr parser doesn't support precedence, so we manually construct it here...
// It'd be nice to bring back explicit precedence via '(' and ')'
// Alternatively, a manual construction could be done with separate bindings for !isMac OR each condition
~condition=
WhenExpr.(
And([
Not(Defined("isMac")),
Or([
And([Defined("editorTextFocus"), Defined("insertMode")]),
Defined("textInputFocus"),
Defined("commandLineFocus"),
]),
])
),
);

let pasteMac =
bind(
~key="<D-V>",
~command=Commands.paste.id,
~condition="isMac" |> WhenExpr.parse,
);
};

module Contributions = {
let commands = [Commands.paste];

let keybindings = Keybindings.[pasteMac, pasteNonMac];
};
5 changes: 4 additions & 1 deletion src/Feature/Clipboard/Feature_Clipboard.rei
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ let update: (msg, model) => (model, outmsg);

module Commands: {let paste: Command.t(msg);};

module Contributions: {let commands: list(Command.t(msg));};
module Contributions: {
let commands: list(Command.t(msg));
let keybindings: list(Feature_Input.Schema.keybinding);
};
2 changes: 1 addition & 1 deletion src/Feature/Clipboard/dune
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(library
(name Feature_Clipboard)
(public_name Oni2.feature.clipboard)
(libraries Oni2.core isolinear base Oni2.feature.commands
(libraries Oni2.core isolinear base Oni2.feature.commands Oni2.feature.input
Oni2.service.clipboard)
(preprocess
(pps ppx_let ppx_deriving.show)))
3 changes: 2 additions & 1 deletion src/Feature/Vim/Feature_Vim.re
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ module Effects = {
};
};

let update = (msg, model: model) => {
let update = (~vimContext, msg, model: model) => {
switch (msg) {
| ModeChanged({allowAnimation, mode, effects, subMode}) => (
{...model, subMode} |> handleEffects(effects),
Expand All @@ -165,6 +165,7 @@ let update = (msg, model: model) => {
| Pasted(text) =>
let eff =
Service_Vim.Effects.paste(
~context=vimContext,
~toMsg=mode => PasteCompleted({mode: mode}),
text,
);
Expand Down
2 changes: 1 addition & 1 deletion src/Feature/Vim/Feature_Vim.rei
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type outmsg =

// UPDATE

let update: (msg, model) => (model, outmsg);
let update: (~vimContext: Vim.Context.t, msg, model) => (model, outmsg);

let getSearchHighlightsByLine:
(~bufferId: int, ~line: LineNumber.t, model) => list(ByteRange.t);
Expand Down
24 changes: 1 addition & 23 deletions src/Model/State.re
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ let defaultKeyBindings =
),
]
@ Feature_SideBar.Contributions.keybindings
@ Feature_Clipboard.Contributions.keybindings
@ Feature_Input.Schema.[
bind(
~key="<C-TAB>",
Expand Down Expand Up @@ -58,29 +59,6 @@ let defaultKeyBindings =
~command=Commands.Workbench.Action.showCommands.id,
~condition="isMac" |> WhenExpr.parse,
),
bind(
~key="<C-V>",
~command=Feature_Clipboard.Commands.paste.id,
// The WhenExpr parser doesn't support precedence, so we manually construct it here...
// It'd be nice to bring back explicit precedence via '(' and ')'
// Alternatively, a manual construction could be done with separate bindings for !isMac OR each condition
~condition=
WhenExpr.(
And([
Not(Defined("isMac")),
Or([
And([Defined("editorTextFocus"), Defined("insertMode")]),
Defined("textInputFocus"),
Defined("commandLineFocus"),
]),
])
),
),
bind(
~key="<D-V>",
~command=Feature_Clipboard.Commands.paste.id,
~condition=isMacCondition,
),
bind(
~key="<ESC>",
~command=Commands.Workbench.Action.closeQuickOpen.id,
Expand Down
25 changes: 19 additions & 6 deletions src/Service/Vim/Service_Vim.re
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,37 @@ let quitAll = () =>
);

module Effects = {
let paste = (~toMsg, text) => {
let paste = (~context=?, ~toMsg, text) => {
Isolinear.Effect.createWithDispatch(~name="vim.clipboardPaste", dispatch => {
let isCmdLineMode = Vim.Mode.isCommandLine(Vim.Mode.current());
let isInsertMode = Vim.Mode.isInsert(Vim.Mode.current());

if (isInsertMode || isCmdLineMode) {
let context =
switch (context) {
| None => Vim.Context.current()
| Some(ctx) => ctx
};
let mode = context.mode;
let isCmdLineMode = Vim.Mode.isCommandLine(mode);
let isInsertMode = Vim.Mode.isInsert(mode);
let isSelectMode = Vim.Mode.isSelect(mode);
let isNormalMode = Vim.Mode.isNormal(mode);
let isVisualMode = Vim.Mode.isVisual(mode);

if (isInsertMode || isCmdLineMode || isSelectMode) {
if (!isCmdLineMode) {
Vim.command("set paste") |> ignore;
};

Log.infof(m => m("Pasting: %s", text));
let (latestContext: Vim.Context.t, _effects) =
Oni_Core.VimEx.inputString(text);
Vim.input(~context, text);

if (!isCmdLineMode) {
Vim.command("set nopaste") |> ignore;
dispatch(toMsg(latestContext.mode));
};
} else if (isVisualMode || isNormalMode) {
let (latestContext: Vim.Context.t, _effects) =
Oni_Core.VimEx.inputString("\"*p");
dispatch(toMsg(latestContext.mode));
};
});
};
Expand Down
4 changes: 3 additions & 1 deletion src/Service/Vim/Service_Vim.rei
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ let saveAllAndQuit: unit => Isolinear.Effect.t(_);
let quitAll: unit => Isolinear.Effect.t(_);

module Effects: {
let paste: (~toMsg: Vim.Mode.t => 'msg, string) => Isolinear.Effect.t('msg);
let paste:
(~context: Vim.Context.t=?, ~toMsg: Vim.Mode.t => 'msg, string) =>
Isolinear.Effect.t('msg);

let getRegisterValue:
(~toMsg: option(array(string)) => 'msg, char) =>
Expand Down
3 changes: 2 additions & 1 deletion src/Store/Features.re
Original file line number Diff line number Diff line change
Expand Up @@ -2251,7 +2251,8 @@ let update =

| Vim(msg) =>
let previousSubMode = state.vim |> Feature_Vim.subMode;
let (vim, outmsg) = Feature_Vim.update(msg, state.vim);
let vimContext = VimContext.current(state);
let (vim, outmsg) = Feature_Vim.update(~vimContext, msg, state.vim);
let newSubMode = state.vim |> Feature_Vim.subMode;

// If we've switched to, or from, insert literal,
Expand Down

0 comments on commit b12edb3

Please sign in to comment.