Differential D12520

[PasteJob] Emit KDirNotify on paste
ClosedPublic
Actions

Authored by broulik on Apr 25 2018, 3:11 PM.

Details

Reviewers
dfaure
Group Reviewers
Frameworks
Commits
R241:681501c276ee: [PasteJob] Emit KDirNotify on paste
Summary

storedPut isn't particularly smart and so after pasting the file tell everyone about it

Test Plan

Copied some text to clipboard, Ctrl+V in Dolphin, typed a name, file showed up immediately. Before that it would only show up after a second

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Apr 25 2018, 3:11 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 25 2018, 3:11 PM
broulik requested review of this revision.Apr 25 2018, 3:11 PM
broulik retitled this revision from [PasteJob] Emit KDirWatch on paste to [PasteJob] Emit KDirNotify on paste.
broulik added a comment.Apr 27 2018, 8:55 AM

Given I've done this twice now, should we perhaps fix storedPut to do that?

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptJun 1 2018, 10:50 AM
elvisangelaccio added a subscriber: elvisangelaccio.Jun 1 2018, 9:09 PM
elvisangelaccio added inline comments.
src/widgets/paste.cpp
129

Missing this as the receiver argument?

broulik added inline comments.Jun 4 2018, 7:54 AM
src/widgets/paste.cpp
129

This is a static so there's no this and we also just access job and then call another static method

ngraham added a subscriber: ngraham.EditedSep 12 2018, 8:21 PM

This patch works well for me, FWIW.

dfaure accepted this revision.Sep 16 2018, 9:09 AM
dfaure added a subscriber: dfaure.

"perhaps fix storedPut to do that?" --> I'm not in favour. Generally speaking, we want to do this at the highest level possible, so that batching N jobs doesn't trigger N notifications.

This is e.g. why CopyJob does it, but FileCopyJob doesn't.

Granted we don't have jobs which do multiple storedPuts right now (IIRC), but I can imagine a day where this is the case.

This revision is now accepted and ready to land.Sep 16 2018, 9:09 AM
Closed by commit R241:681501c276ee: [PasteJob] Emit KDirNotify on paste (authored by broulik). · Explain WhySep 18 2018, 8:46 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/widgets/paste.cpp (6 lines)

Diff 41885

View Options

src/widgets/paste.cpp

Loading...