vim mode 'r <Enter>' keeps replace mode active #1246

Closed
opened 2013-02-14 01:23:46 +01:00 by ghost1 · 9 comments
ghost1 commented 2013-02-14 01:23:46 +01:00 (Migrated from gitlab.com)

I use 'r ' to split lines and when I do so, my next keypress is taken as the character to use in the replace. The is also used, but replace mode remains active.

I use 'r <Enter>' to split lines and when I do so, my next keypress is taken as the character to use in the replace. The <Enter> is also used, but replace mode remains active.
marijnh commented 2013-02-14 01:32:41 +01:00 (Migrated from gitlab.com)

Well, I'm seeing that typing 'r ' replaces the current character with the word 'Space', which is definitely a bug. I don't understand what you mean by using 'r ' to split lines though. Could you provide a more concrete example?

Well, I'm seeing that typing 'r ' replaces the current character with the word 'Space', which is definitely a bug. I don't understand what you mean by using 'r ' to split lines though. Could you provide a more concrete example?
ghost1 commented 2013-02-14 01:44:46 +01:00 (Migrated from gitlab.com)

sorry, yes.

'r ' replaces the character with Space, 'r' replaces the character with the word 'Down', etc. 'r' should replace the current character with a newline, which it does, but should also end replace mode.

For example:

hello world

cursor is on the space between 'hello' and 'world', typing 'ri' should leave the buffer as

hello
world

and insert mode should be active. Instead, we get

hello
iorld

sorry, yes. 'r ' replaces the character with Space, 'r<Down>' replaces the character with the word 'Down', etc. 'r<Enter>' should replace the current character with a newline, which it does, but should also end replace mode. For example: hello world cursor is on the space between 'hello' and 'world', typing 'r<Enter>i' should leave the buffer as hello world and insert mode should be active. Instead, we get hello iorld
marijnh commented 2013-02-14 01:54:46 +01:00 (Migrated from gitlab.com)

Got it, github is stripping out your <Enter>, you should enclose it with backticks. So you are saying r<Enter> should replace the current character with \n. I agree and I think I have an idea of how to fix that.

But could you type out again the key sequence that results in

hello
iorld

Doing r<Enter>i would get me helloEnterworld, with the cursor before the E, in insert mode. No newline is inserted.

Got it, github is stripping out your `<Enter>`, you should enclose it with backticks. So you are saying `r<Enter>` should replace the current character with `\n`. I agree and I think I have an idea of how to fix that. But could you type out again the key sequence that results in hello iorld Doing `r<Enter>i` would get me helloEnterworld, with the cursor before the E, in insert mode. No newline is inserted.
ghost1 commented 2013-02-14 02:00:05 +01:00 (Migrated from gitlab.com)

ah, sorry for the confusion. yes, so r<Enter>i does result in helloEnterworld in the vim keybindings demo on the codemirror site.

In the application I am using that uses codemirror the same keystrokes lead to \n being inserted instead of Enter but the i is then inserted in place of the next character. Perhaps fixing the entering of special characters as words will help both those cases...

ah, sorry for the confusion. yes, so `r<Enter>i` does result in helloEnterworld in the vim keybindings demo on the codemirror site. In the application I am using that uses codemirror the same keystrokes lead to `\n` being inserted instead of `Enter` but the `i` is then inserted in place of the next character. Perhaps fixing the entering of special characters as words will help both those cases...
marijnh commented 2013-02-14 15:58:02 +01:00 (Migrated from gitlab.com)

@mightyguava I've added a few changes at vim.js:598 for that in my personal version of CodeMirror (not the one on github though), and planned to commit those here once I've got the current additions stable. In my version I actually explicitely catch the keys from specialKeys that have printable equivalents (Space and Enter right now). My code doesn't do anything with the others, but probably it should do something 'bell'-like (maybe we should open an issue about how to deal with these errors in general). Also in my code r<CR> leaves the cursor at the start of the new linebreak (as in 'behind the last character of the line') which I need to fix. I'd like to sort out the selection-issues first though, since a more significant change I made to r is allowing it to operate on selections. For that again I had to write a test first that expected wrong results, due to selection being one character short.

`@mightyguava` I've added a few changes at [vim.js:598](https://github.com/metatheos/CodeMirror/blob/58e56150cb750840ddb27ed89a46c2d61819ef9e/keymap/vim.js#L598) for that in my personal version of CodeMirror (not the one on github though), and planned to commit those here once I've got the current additions stable. In my version I actually explicitely catch the keys from `specialKeys` that have printable equivalents (Space and Enter right now). My code doesn't do anything with the others, but probably it should do something 'bell'-like (maybe we should open an issue about how to deal with these errors in general). Also in my code `r<CR>` leaves the cursor at the start of the new linebreak (as in 'behind the last character of the line') which I need to fix. I'd like to sort out the selection-issues first though, since a more significant change I made to `r` is allowing it to operate on selections. For that again I had to write a test first that expected wrong results, due to selection being one character short.
marijnh commented 2013-02-14 16:21:57 +01:00 (Migrated from gitlab.com)

@metatheos Awesome! I was planning to tackle this today but you got to it first :) You may need to special case <CR> in the replace motion, since you'll probably want to use newlineAndIndentContinueComment to make the newline instead of \n

Yes... visual mode handling is quite inelegant since CodeMirror's selection model is a little bit different from what would be expected of Vim. I believe I added a hack to include the last character in visual mode here github.com/metatheos/CodeMirror@58e56150cb/keymap/vim.js (L875). Is it not working for you?

I also thought you were planning in https://github.com/marijnh/CodeMirror/pull/1235#issuecomment-13551933 to keep a separate selection model and emulate CodeMirror selections with setSelection, which can go a long way to make this better. Or is that for later?

`@metatheos` Awesome! I was planning to tackle this today but you got to it first :) You may need to special case `<CR>` in the replace motion, since you'll probably want to use newlineAndIndentContinueComment to make the newline instead of `\n` Yes... visual mode handling is quite inelegant since CodeMirror's selection model is a little bit different from what would be expected of Vim. I believe I added a hack to include the last character in visual mode here https://github.com/metatheos/CodeMirror/blob/58e56150cb750840ddb27ed89a46c2d61819ef9e/keymap/vim.js#L875. Is it not working for you? I also thought you were planning in https://github.com/marijnh/CodeMirror/pull/1235#issuecomment-13551933 to keep a separate selection model and emulate CodeMirror selections with setSelection, which can go a long way to make this better. Or is that for later?
marijnh commented 2013-02-14 17:02:06 +01:00 (Migrated from gitlab.com)

you'll probably want to use newlineAndIndentContinueComment

You're right, I definitely have to adapt that for consistency. Vim-help actually states that it will insert only one linebreak if <CR> or <NL> is the replacement character for a given range, so this is a special case anyway.

Is it not working for you?

I would need to check why it didn't work. I also could have added the same hack to r of course, but at that point I was curious whether it would work out to improve visual mode either way ;)

I also thought you were planning in #1235 to keep a separate selection model and emulate CodeMirror selections with setSelection, which can go a long way to make this better. Or is that for later?

I would prefer to maybe add optional from/to-arguments to setSelection if codemirror can reliably deal with that in general. Locally I do have an implementation that feels a lot more clean, save the new vim.js-specific setSelection that is pretty much:

function setSelection(cm, vim, curStart, curEnd, linewise){
  cm.operation(function(){
    var from, to;
    {do the same as cm.setSelection}
    if(linewise){
      from.ch=0;
      to.ch=lineLength(cm, to.line);
    }else{
      to.ch++;
    }
    sel.from=cm.clipPos(from);
    sel.to=cm.clipPos(to);
  })
}
> you'll probably want to use newlineAndIndentContinueComment You're right, I definitely have to adapt that for consistency. Vim-help actually states that it will insert only one linebreak if `<CR>` or `<NL>` is the replacement character for a given range, so this is a special case anyway. > Is it not working for you? I would need to check why it didn't work. I also could have added the same hack to `r` of course, but at that point I was curious whether it would work out to improve visual mode either way ;) > I also thought you were planning in #1235 to keep a separate selection model and emulate CodeMirror selections with setSelection, which can go a long way to make this better. Or is that for later? I would prefer to maybe add optional `from`/`to`-arguments to `setSelection` if codemirror can reliably deal with that in general. Locally I do have an implementation that feels a lot more clean, save the new vim.js-specific `setSelection` that is pretty much: ``` function setSelection(cm, vim, curStart, curEnd, linewise){ cm.operation(function(){ var from, to; {do the same as cm.setSelection} if(linewise){ from.ch=0; to.ch=lineLength(cm, to.line); }else{ to.ch++; } sel.from=cm.clipPos(from); sel.to=cm.clipPos(to); }) } ```
marijnh commented 2013-02-18 21:25:48 +01:00 (Migrated from gitlab.com)

mentioned in merge request !8420

mentioned in merge request !8420
marijnh commented 2013-03-01 03:32:23 +01:00 (Migrated from gitlab.com)

This is fixed by the above change by @metatheos. Please verify and close.

This is fixed by the above change by `@metatheos.` Please verify and close.
marijnh (Migrated from gitlab.com) closed this issue 2013-03-01 09:19:03 +01:00
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
codemirror/codemirror5#1246
No description provided.