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

Commands p, P, u fixed/added and clipboard selection type is tracked #16

Merged
merged 5 commits into from
Dec 21, 2016

Conversation

pwoosam
Copy link
Collaborator

@pwoosam pwoosam commented Dec 6, 2016

Fixes #13


Command u updates cursor after undo.
Command p fixed
Command P added

Also I believe there is another bug in u. The normal undo behavior in vim appears to set the cursor to the position of cursor before the last change took place, not the position of the last change.

Issue spyder-ide#13 fixed.
Command u updates cursor after undo.
Command p fixed
Command P added
P now pastes single lines above current line.
Both p and P work with repeat argument now.
p and P would show the cursor one space to the right of the first word
if the clipboard contained a single line of non-indented text.

Now, p and P place the cursor at the beginning of the first word when
pasting non-indented, single lines
@ccordoba12 ccordoba12 added this to the v0.1 milestone Dec 9, 2016
@ccordoba12 ccordoba12 changed the title issue 13 fixed and commands p, P, u fixed/added Yanking now puts the line on a new line and commands p, P, u fixed/added Dec 9, 2016
@pwoosam
Copy link
Collaborator Author

pwoosam commented Dec 14, 2016

@Nodd This kind of works for now, but it'll have some issues with pasting multiple lines as a block since it doesn't see it as a block selection. It treats it as a character selection instead. I believe vim had its own clipboard that kept track of the different types of selections (block, line, and character).

@Nodd
Copy link
Contributor

Nodd commented Dec 14, 2016

vim has to work with external copy/paste too (typically, middle click of the mouse on Linux), I don't know how it's managed. It should be possible to record internally the type of selection when pasting?
Anyway I'm not a regular vim user so you know better than me how it works!

@pwoosam
Copy link
Collaborator Author

pwoosam commented Dec 14, 2016

It handles the the external copy/paste as if they were a selection of characters. So it would have the same behavior as this PR does when it pastes multiple lines.

I'm not sure what the best way of keeping track of the selection type since QClipboard can't be subclassed.

I suppose a class with two properties, one clipboard and one string to keep track of the type would work?

Actually, that's unnecessarily complicated. We don't need to copy the clipboard, just keep track of the type when it is copied by vim. Just have VimWidget have a single property to keep track of the type.

@pwoosam pwoosam force-pushed the command-p-P branch 2 times, most recently from 98246b7 to 163a510 Compare December 15, 2016 12:16
@pwoosam
Copy link
Collaborator Author

pwoosam commented Dec 15, 2016

@Nodd, check out the latest commit. It keeps track of the selection type now and has the same vim behaviors for p and P that you'd expect. 👍

@pwoosam pwoosam changed the title Yanking now puts the line on a new line and commands p, P, u fixed/added Commands p, P, u fixed/added and clipboard selection type is tracked Dec 15, 2016
VimWidget now keeps track of the clipboard and selection type.
P and p will take selection type into account when pasting text.
@pwoosam
Copy link
Collaborator Author

pwoosam commented Dec 17, 2016

@Nodd I also have implemented visual mode and some of its commands. But this PR should be merged first to take advantage of visual mode.

@pwoosam pwoosam mentioned this pull request Dec 19, 2016
@ccordoba12
Copy link
Member

@pwoosam, I invited you to be collaborator on this repo. This means you'll be able to tag, edit, and merge/close issues and pull requests.

Since you're very interested in the progress of spyder-vim, I want to trust on you and give you the freedom to keep working on this without waiting for @Nodd to review/merge your work.

So feel free to merge this and the other PRs you have submitted whenever you consider they are ready. I'm sure @Nodd will leave you comments in case he finds it necessary. For that, I only recommend you to always create PRs for your work, instead of submitting it directly to master.

Of course, if you want me to review your work, don't hesitate to ping me (although my knowledge of Vim is almost zilch ;-)

@ccordoba12
Copy link
Member

And don't forget to mark the commands you implement on each PR on issue #1 ;-)

@pwoosam
Copy link
Collaborator Author

pwoosam commented Dec 21, 2016

@ccordoba12 Thank you for inviting me to be a collaborator on this repo! I'll be sure to ping you if I need any help reviewing my work.

It's always a pleasure to contribute :)

@pwoosam pwoosam merged commit ee3aab1 into spyder-ide:master Dec 21, 2016
@pwoosam pwoosam deleted the command-p-P branch December 21, 2016 06:43
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.

When you yank a whole line, p (for "put" or "paste") should put the line on a new line.
3 participants