Add support for move operation #93
Conversation
|
Ready for review |
| webDAVFS.moveEntry({ | ||
| sourcePath: options.sourcePath.substring(1), | ||
| targetPath: options.targetPath.substring(1), | ||
| onSuccess: function() { |
There was a problem hiding this comment.
This can be simply written as onSuccess: onSuccess
There was a problem hiding this comment.
Good point. I've used this redundant pattern throughout the file though, can I have another PR to tidy up the whole file?
There was a problem hiding this comment.
Created issue at #97
|
Are we gonna work on those two points in other PRs then? Are there any other issues here? |
|
I think some of #21 are apparent, so you could add this now (and fix the mapping table if we want to modify it). |
|
okay will do |
|
Alright, got the new error reporting in. |
| * @return {string} The FSP error code. | ||
| */ | ||
| var getError = function(code) { | ||
| code = '' + code; |
There was a problem hiding this comment.
historically, I think you can simply write as:
201: 'OK', 204: 'OK', ...
for Object literals, where each keys are string.
And if you specify a number to the object, it's converted into a string. So you don't have to cast like this here, and you can omit the quote for keys in line 16-24. I believe that's more concise.
There was a problem hiding this comment.
Yeah, I usually use the implicit conversion but I was trying to be more rigorous here. It is more concise, though, so I'm happy with that.
|
cool, lgtm. |
|
hm, but something prevents the automatic merge. Could you rebase the PR? |
|
rebased |
|
looks failing |
|
Oh it's because I changed the expected error string in the tests for WebDAV. I've updated it to the correct string here, I will fix it more robustly in a future PR that adds proper error handling to S3 like I did for WebDAV |
|
cool fixed |
|
cool |
This implements non-recursive move for S3 and WebDAV.