GNOME Bugzilla – Bug 740383
Allow items on Google Drive to be renamed
Last modified: 2014-11-21 13:12:01 UTC
I have been working on a GVfs backend for Google Drive (see bug 739008). The semantics of Google Drive and the implementation of the rename operation in Nautilus don't work together. The path to a file or folder (ie. a GDataDocumentsEntry) on Google Drive is composed of their document-ids. See: https://developer.gnome.org/gdata/unstable/GDataDocumentsEntry.html#gdata-documents-entry-get-path These document-ids are not human readable and not what is presented in the UI. In other words, the document-id is the standard::name, while the title of the GDataDocumentsEntry makes up the standard::display-name. When a rename happens, the SetDisplayName method is invoked on the GVfs backend which changes the title of the entry, but the document-id usually remains the same. See: https://developer.gnome.org/gdata/unstable/GDataEntry.html#gdata-entry-set-title This breaks the logic in nautilus' rename operation because it expects both standard::name and standard::display-name to change, which is usually what happens on other backends. Since the standard::name of the file before and after the operation is the same, it thinks that there is some other file by the same name which would get overwritten and hides it. But in reality, it is the same file that we are renaming, and not some other file. I propose to tweak the logic a bit to accommodate this scenario. As far as I can see, this does not adversely affect other file systems. I tried a local UNIX file system and a WebDAV mount. The backends correctly return a G_IO_ERROR_EXISTS if you try to give a name to a file that is already taken, and the operation errors out gracefully. So, we are probably not going to hit the above logic in nautilus for the other backends anyway.
Created attachment 291026 [details] [review] file: Let renames work on Google Drive
Created attachment 291027 [details] [review] file: Remove unused variable
So, if any backend except the google one is actually using that logic because g_file_set_display_name_async raises an error in those cases, why don't you just raise an error in g_file_set_display_name_async for google backend case as well and we can get rid of that logic in nautilus?
(In reply to comment #3) > So, if any backend except the google one is actually using that logic because > g_file_set_display_name_async raises an error in those cases, why don't you > just raise an error in g_file_set_display_name_async for google backend case This logic in the rename operation is based on standard::name, and for Google Drive it is impossible for any two entities to have the same standard::name because they are document-ids generated by the server. So, we will never encounter a scenario where we are renaming a file and its standard::name after the operation is the same as another file, in which case we would have had to hide the other file. In other words, the uniqueness of the standard::name is guaranteed by the server and we don't need to raise G_IO_ERROR_EXISTS. However, we do encounter the case where the standard::name of the file remains unchanged before and after rename operation because that is how the Google Drive backend works - only the standard::display-name is changed. This causes nautilus to think that the 'new' standard::name is the same as that of another file and it needs to hide it. In reality, there isn't any other file with the same standard::name. It is the same file that we are renaming, so we should not hide anything. > we can get rid of that logic in nautilus? We can, but I don't know nautilus well enough to know whether it will have any negative consequences.
Ok I see, I would like in any case to get rid of that logic. I would let Cosimo take a look here since I don't know enough to make a decision neither. If that logic is not needed, delete that part and your patch is not needed. If it is needed, then your patch looks ok.
Review of attachment 291027 [details] [review]: OK
Review of attachment 291026 [details] [review]: I think this is a reasonable patch. That check has been there for a very long time (commit 6194a4b0) and I'd rather leave it in.
Thanks for taking a look, Cosimo.