Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sane defaults for movement key remaps #50

Merged
merged 1 commit into from
Nov 15, 2019
Merged

Sane defaults for movement key remaps #50

merged 1 commit into from
Nov 15, 2019

Conversation

jkav77
Copy link
Contributor

@jkav77 jkav77 commented Jul 22, 2016

Change the remapping of movement keys when Pencil mode is enabled tocheck for user remaps and use those as a sane default.

Fixes #49

check for user remaps and use those as a sane default.

Fixes #49
@jkav77
Copy link
Contributor Author

jkav77 commented Dec 20, 2016

Any reason not to merge this? Can I change anything to make it acceptable?

@reedes
Copy link
Collaborator

reedes commented Dec 21, 2016

Sorry for the delay. I haven't had a chance to spend much time with the plugins and will get to this and other proposed changes in next few weeks.

@stu0292
Copy link

stu0292 commented Oct 2, 2018

any update on this?

@alerque
Copy link
Member

alerque commented Nov 15, 2019

Thanks for taking the time to contribute this @dangerginger. I've recently stepped in to help maintain some of Reedes's prose related plugins and have had a gander at this change. It looks sane to me. If you (or @stu-b-doo) are still around and using this it would be great to have a confirmation that it works as expected with your bindings after I merge. I use a remapped keyboard (a dvorak variant) but actually kept the default j/k bindings, so while I can artificially test this it doesn't actually change anything for me in practice. It would be nice to hear that it does the right thing for folks who used various different ways of rebinding j/k motions.

@alerque alerque merged commit 907d26e into preservim:master Nov 15, 2019
@davidsierradz
Copy link

Hi guys, for some reason this is not working for me, I'm using neovim NVIM v0.5.0-159-ge3b08a0fc with this minimal init.vim:

function! Mapkey (keys, mode) abort
    " Pass in a key sequence and the first letter of a vim mode.
    " Returns key mapping mapped to it in that mode, else 0 if none.
    " example:
    "   :nnoremap <Tab> :bn<CR>
    "   :call Mapkey(':bn<CR>', 'n')
    "   " returns <Tab>
    redir => mappings | silent! map | redir END
    for map in split(mappings, '\n')
        let seq = matchstr(map, '\s\+\zs\S*')
        if maparg(seq, a:mode) == a:keys
            echom string(seq)
            return seq
        endif
    endfor
endfunction

exe 'nn <buffer> <silent> ' . Mapkey('j', 'n') . ' gj'

" '0'
echom Mapkey('gj', 'n')

This maps 0 -> gj, should be j -> gj, right? thanks.

@pfheatwole
Copy link

This maps 0 -> gj, should be j -> gj, right?

Correct. This commit breaks the 0 mapping for "start of line".

Judging by the comment in Mapkey, it looks like 0 is a default return value for "no mappings", but the changed lines: https://github.com/reedes/vim-pencil/blob/c847322db8a15fc4513dfaf16623004d210d3a99/autoload/pencil.vim#L411-L414

are treating the 0 as if it was the mapping for movement keys.

@alerque
Copy link
Member

alerque commented Nov 16, 2019

Thanks for the report @davidsierradz and @pfheatwole, that's a nasty little regression that I didn't notice. I'm in the habit of using ^ not 0 to go to the first non-whitespace character in a line and almost never even think of 0. If '0' is the default being turned up when asked if any mapping exists rather than another way of saying 'false' then we need to check for it specifically. I'll look into that right now.

@alerque
Copy link
Member

alerque commented Nov 16, 2019

Hey @davidsierradz and @pfheatwole the regression you encountered should be fixed now. I also touched up a couple other little details such as mapping g0 and g$ to what used to be 0 and $ when enabling Pencil — much like gj was mapped to j (basically flipping g/non-g variants). Previously 0 and $ were being remapped but their original counterparts were no longer accessible at all.

There are quite a few more things in this department that need some attention. See my notes in #83 and feel free to pitch in with any ideas or concerns. Sorry again about messing this up for you and thanks for the reports.

@pfheatwole
Copy link

Thanks for your work! I appreciate you helping with maintenance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pencil Mode breaks remapped movement keys
6 participants