Differential D15318

Automatically re-upload saved files located on remote locations instead of asking first
AbandonedPublic
Actions

Authored by ngraham on Sep 6 2018, 4:08 PM.

Details

Reviewers
broulik
jtamate
Group Reviewers
Frameworks
Summary

Right now, when you use a non-KDE app to save an edited file that's located on a remote location share, KIO annoyingly asks whether you'd like to re-upload it it after every save. If you say yes, it then presents you with an overwrite confirmation dialog.

This is a non-optimal user experience that is not consistent with the local file save experience and exposes implementation details. Therefore, automatically re-upload the saved file with no confirmation just like the user expects.

TODO: test with ftp, sftp, nfs, fish, and gdrive ioslaves

BUG: 398216
FIXED-IN: 5.51

Test Plan

Put a LibreOffice document on a Samba share.

  1. Change the file in LibreOffice and save the file. The file is automatically re-uploaded to the server after save, just like one would expect it to.
  1. Change the file in LibreOffice, but before saving it, delete or rename the file on the Samba share.
  2. Observe that KIO notices and alerts you:
    Save Nevertheless saves the file anyway, and Save Somewhere Else displays a KDE file picker at the original location that allows you to save it elsewhere. Both work as expected.

Put a very large GIMP document on a Samba share.

  1. Open the file, edit it, and save it.
  2. Wait for the file to finish saving, and then *immediately* edit it and save again (i.e. while the first save is still uploading)
  3. Close and open the document again
  4. Observe that the second set of edits were saved to the file on the Samba share

Diff Detail

Repository
R241 KIO
Branch
samba-auto-re-upload-after-save (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2922
Build 2940: arc lint + arc unit
ngraham created this revision.Sep 6 2018, 4:08 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 6 2018, 4:08 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.Sep 6 2018, 4:08 PM
broulik added a comment.Sep 6 2018, 4:12 PM

I would like to have some sanity checks there, like if the file is empty, or its mime type changed, it should ask. Also what if the file changes, is uploaded, and then changes again midflight? I'm a person that mashes Ctrl+S usually.

ngraham planned changes to this revision.Sep 6 2018, 4:25 PM

That makes sense. I can add the mimetype check. For the save mashing case, do you have a proposal?

jtamate added a comment.Sep 6 2018, 4:43 PM

It does automatically upload the modified file, but wouldn't it be nicer to not get the dialog to overwrite the modified file, that of course is not there in the case of local files.
It is just a third parameter to copy and tell it to Overwrite.

ngraham added a comment.Sep 6 2018, 4:56 PM
In D15318#321460, @jtamate wrote:

It does automatically upload the modified file

Not before asking, which is what this patch takes out.

but wouldn't it be nicer to not get the dialog to overwrite the modified file

You're still seeing that with git master? If so, I agree that we should fix that too.

jtamate added a comment.Sep 6 2018, 5:09 PM
In D15318#321461, @ngraham wrote:

but wouldn't it be nicer to not get the dialog to overwrite the modified file

You're still seeing that with git master? If so, I agree that we should fix that too.

Yes, without your patch, every time I agree to upload the modified file then I have to agree to overwrite the remote file.

elvisangelaccio added a subscriber: elvisangelaccio.Sep 6 2018, 9:09 PM

Note that kioexecd is slave-agnostic, we shouldn't test only with the samba slave.

ngraham retitled this revision from Automatically re-upload saved files located on samba shares instead of asking first to [WIP] Automatically re-upload saved files located on samba shares instead of asking first.Sep 10 2018, 3:23 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham updated this revision to Diff 41305.Sep 10 2018, 3:24 AM
  • Don't show the unnecessary and annoying overwrite dialog
  • Make a note of the validation steps I need to implement
ngraham planned changes to this revision.Sep 10 2018, 3:28 AM
ngraham retitled this revision from [WIP] Automatically re-upload saved files located on samba shares instead of asking first to [WIP] Automatically re-upload saved files located on remote locations instead of asking first.
ngraham updated this revision to Diff 41370.Sep 10 2018, 10:39 PM

Implement first pass at some user-friendly error checking

ngraham planned changes to this revision.Sep 10 2018, 10:39 PM
ngraham edited the summary of this revision. (Show Details)Sep 10 2018, 11:29 PM
ngraham edited the summary of this revision. (Show Details)Sep 11 2018, 3:28 AM
ngraham edited the test plan for this revision. (Show Details)
ngraham updated this revision to Diff 41374.Sep 11 2018, 3:28 AM

Adjust message string

ngraham planned changes to this revision.Sep 11 2018, 3:28 AM
ngraham updated this revision to Diff 41718.Sep 16 2018, 12:02 AM
  • Only allow one upload job at a time; if one is already going, restart it
ngraham retitled this revision from [WIP] Automatically re-upload saved files located on remote locations instead of asking first to Automatically re-upload saved files located on remote locations instead of asking first.Sep 16 2018, 12:05 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham planned changes to this revision.Sep 16 2018, 1:16 AM
ngraham updated this revision to Diff 41749.Sep 16 2018, 4:53 PM

Fix some compile errors, oops :/

ngraham planned changes to this revision.Sep 16 2018, 4:53 PM
maverick74 added a subscriber: maverick74.Apr 8 2019, 4:34 PM

@ngraham What's the state of this?

ngraham added a comment.Apr 8 2019, 6:24 PM

Thanks for the poke, I'll try to revive this.

maverick74 added a comment.Apr 9 2019, 8:51 AM

That would be great!
It looks very bad the way it is ;)

ngraham abandoned this revision.Nov 29 2019, 6:59 PM

This is rendered moot by the far superior FUSE-based solution at D23384!

Revision Contents
Changeset List

PathPackages
Msrc/kioexec/kioexecd.h (3 lines)
Msrc/kioexec/kioexecd.cpp (57 lines)

Diff 41749

View Options

src/kioexec/kioexecd.h

Loading...
View Options

src/kioexec/kioexecd.cpp

Loading...