Skip to content

Mapping improvements and cleanup #2

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Mapping improvements and cleanup #2

wants to merge 6 commits into from

Conversation

noahfrederick
Copy link

Hi,

I made a few adjustments to your plug-in involving, most importantly, making the maps configurable. See commit messages for the specifics. I'm opening this pull request to share if interested, but feel free to ignore.

I'll likely be making more changes later such as restoring registers within functions and using getpos()/setpos() instead of marks.

- Stripped trailing spaces
- Replaced mixed indentation styles with one
- Use consistent command abbreviations
- Use ! with normal where appropriate
- Use nore- mappings where appropriate
- Other misc. formatting
- Only set buffer-local mappings
- Use <LocalLeader> rather than hardcoding "\"
- Use script-local functions with <Plug> maps
- Only set default maps if none already exist
Users likely already have an ftplugin/php.vim file
- Ensure property is placed in containing class, not next class
- Do not assume opening bracket is on next line
@kagux
Copy link

kagux commented Oct 14, 2013

@beberlei, is there any reason why you don't want to merge this PR?

@stevenklar
Copy link

Why using nmap instead of nnoremap?
Would be more safe to stay at nnoremap.

Also for "extractMethod" this should be used.

Regards,
Steven.

@noahfrederick
Copy link
Author

@stevenklar,

Why using nmap instead of nnoremap?
Would be more safe to stay at nnoremap.

Look again at the righthand side of the expression. Those <Plug> maps are maps, which won't work with the nore variants of map. There's nothing unsafe about it—this is what <Plug> is for.

—N

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

Successfully merging this pull request may close these issues.

3 participants