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

Add support for copy on Windows systems #231

Merged
merged 2 commits into from
Aug 26, 2016

Conversation

SkylerLipthay
Copy link
Contributor

I've added support for copying files and directories on Windows systems through the copy and xcopy DOS commands. I also fixed a bug for the success check in the Path.copy function by replacing success != 0 with v:shell_error != 0; I believe this fix represents the intended functionality.

My code has been tested on Windows XP SP3, Windows 8, and Mac OS X 10.6. Mac (Unix) functionality has remained unchanged.

@PhilRunninger
Copy link
Member

@SkylerLipthay, I like this. It fills a hole in the Windows side of the code. Fix the merge conflicts, and I'll merge it in. Thanks for contributing.

@SkylerLipthay
Copy link
Contributor Author

@PhilRunninger I've rebased from upstream and force pushed to my fork. I cannot test this functionality anymore, as I unfortunately don't have easy access to a Windows machine. Also, my Vimscript is pretty shoddy, so while everything looks good to me, I might be overlooking something obvious.

Side note: this was my first public OSS contribution, so I'm glad to see it get some closure! :) Thank you for your work.

@PhilRunninger PhilRunninger merged commit 7730c87 into preservim:master Aug 26, 2016
@PhilRunninger
Copy link
Member

I had to make a few minor changes to your PR, but overall it was good. Thanks for the update.

@symphony2014
Copy link

But it still does not work on my WIN10. It always remind me :

NERDTree: Could not copy node.

image

@PhilRunninger
Copy link
Member

@symphony2014, please open a new issue with all relevant details filled in so that we can reproduce your issue.

Thanks.

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