ayurmedia / atom-sync-cygwin

Atom package to sync files bidirectionally between remote host and local over ssh+rsync optimized for Windows with cygwin
https://atom.io/packages/atom-sync-cygwin
MIT License
5 stars 5 forks source link

fix sync upload destination folder doesn't have / #2

Closed mean-cj closed 7 years ago

mean-cj commented 7 years ago

fix sync upload destination folder doesn't have (/)

ayurmedia commented 7 years ago

Upload worked for me, i have to check why this is needed.

mean-cj commented 7 years ago

follow https://github.com/dingjie/atom-sync/issues/71

ayurmedia commented 7 years ago

Makes sense. I will test your changes and then pull it.

ayurmedia commented 7 years ago

i tested and see no difference (at least on osx), i pulled your request, as the comments in the atom-sync feedback make sense. if i see odd behaviour i might revert the bugfix. in rsync the trailing slashes are very important and one should not experiment with them. the safest combination is to use trailing slash on local and remote. if used wrong one creates a folder inside the folder. the syntax of "rsync" is different from "cp" !! The syntax with trailing slash seems to be correct. because otherwise the folder could be created as a file if the folder didn't exist yet.

i also noticed in the code there is sometimes

this should be formatted in the same way. probably always path.sep. because of cygwin its not so important because we always use unix style "forward-slash".

i have another problem with creating new files, because atom-sync console shows an error. but after saving it and opening it again. or simply saving it 2 times it shows green correct. seems there is still a bug in "create file" behaviour. also creating folder or moving folders is quite buggy. (workaround for me is to create folders on server and then sync it down to local).

ayurmedia commented 7 years ago

1) Create New File:

orange:

receiving incremental file list
sent 80 bytes  received 141 bytes  442.00 bytes/sec
total size is 0  speedup is 0.00
Partial transfer due to error, please review your config file.

2) Save same file again:

green:

receiving incremental file list
sent 83 bytes  received 57 bytes  93.33 bytes/sec
total size is 4  speedup is 0.03
Sync completed without error.

clearly not the expected behaviour.

Problem seems to be that a "create new file" does not trigger a "save" event, so "onDidSave" is not triggered and the file is not uploaded to the server. Until the new file is saved manually it will not exist on the server and show the "partial transfer" error .

But a new file will create a new buffer in atom, and try to download the file to check if there is a newer version on the server. But there is no file on the server so the download gets confused and shows the "partially transferred error" (eg. local has a file which should not be there).

The error is triggered by " syncDownOnOpen: true" which will download the file on "create new", but it was never uploaded.

After saving the new file (even empty) the problem disappears.

So partially the error message is correct, because it hints that the file is not yet uploaded to the server and only lives as a unsaved files in the atom-buffer.

To make the user-experience better one could improve it by: 1) automatically upload the file on "create new" (which locally exists in the filesystem), so one might assume it automatically gets uploaded. This could be done with a trick, by listening for "onOpen" and do some special comparison and upload it on demand if it is missing on the remote server.

onOpen: (obj) ->
    config = @config.load obj
    @onSync obj, 'down' if config?.behaviour?.syncDownOnOpen

2) or the Error Message "partially upload error" is intercepted and an alternative message is given to remind the user not to forget to save the file (again).

mean-cj commented 7 years ago

Thank you very much.