Skip to content

README updated for more options - #17

Open
lucaspbordignon wants to merge 2 commits into
christoomey:masterfrom
lucaspbordignon:tips-readme
Open

README updated for more options#17
lucaspbordignon wants to merge 2 commits into
christoomey:masterfrom
lucaspbordignon:tips-readme

Conversation

@lucaspbordignon

Copy link
Copy Markdown

No description provided.

@christoomey christoomey left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lucaspbordignon, thanks for the updates. I had a handful of comments and in the end I think the only one we'll want to keep is the section related to Plug.

Comment thread README.md Outdated
nmap y <Plug>SystemCopy
" Mapping p to use paste
nmap p <Plug>SystemPaste
```

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to setting your Vim clipboard to use the system clipboard. This is actually the core thing I wanted to avoid as I prefer to keep Vim & system clipboard separate. This plugin exists to provide an additional operator for explicitly interacting with the system clipboard. Can you remove this section?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right, that's a good point, I only wanted to add this info on the README as one may want to customize the keys to copy and paste through vim-system-copy and they don't know how to. #14 talks about it. I could change the y or p keys to the same example on this issue (Y and P), what do you think?

Comment thread README.md Outdated
install the plugin:

Add the following line to your `~/.vimrc` and then run `:PluginInstall` from
Add the following line to your `.vimrc` and then run `:PluginInstall` from

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you made the change here, but ~/.vimrc was very much intended as I try to be as explicit as possible with OSS and READMEs. ~/.vimrc is the full path, whereas .vimrc is a relative path that won't work unless you're in the home directory. Mind removing this as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I simply put that way because someone could have a custom .vimrc path. I'll fix that!

Comment thread README.md
" ...
Plug 'christoomey/vim-system-copy'
" ...
call plug#end()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a plug user myself these days, so I'd be fine with removing the Vundle section and just replacing with thus plug reference. Mind making that change?

Comment thread README.md Outdated
```

An alternative would be to use [Plug](https://github.com/junegunn/vim-plug).
As done with Vundle, simply add to your `.vimrc`:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the above summary, can you make this ~/.vimrc instead of .vimrc?

@lucaspbordignon

Copy link
Copy Markdown
Author

@christoomey thanks for the attention! I did the requested changes and made some comments on them, feel free to reply them! 😀

Comment thread README.md Outdated
nmap Y <Plug>SystemCopy
" Mapping P to use paste
nmap P <Plug>SystemPaste
```

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of documenting custom key bindings, but to revisit what I said in my first comment, the goal is to avoid replacing y and p. Can you rewrite this in terms of a leader mapping? Something like:

If you'd rather not use the default mappings, you can configure
custom mappings instead:

``` vim
# Use <leader> maps instead of the default key maps
nmap <leader>cp <Plug>SystemCopy
nmpa <leader>cv <Plug>SystemCopy

@lucaspbordignon lucaspbordignon Apr 24, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I think I mess some concepts myself, you're completely right, sorry for that. Fixed this on e9fb675

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.

2 participants