D29006: Allow to copy or move selection to the other split view

This revision was automatically updated to reflect the committed changes. Closed by commit R318:a291c5999035: Allow to copy or move selection to the other split view (authored by aprcela, committed by ngraham). REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29

D29006: Allow to copy or move selection to the other split view

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Thank you! REPOSITORY R318 Dolphin BRANCH arcpatch-D29006 REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yur

D29006: Allow to copy or move selection to the other split view

aprcela marked an inline comment as done. aprcela added a comment. Done :) REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, waitquietly, azyx,

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 82883. aprcela marked 2 inline comments as done. aprcela added a comment. - Renaming of functions REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=82880&id=82883 BRANCH arcpatch-D29006 REVISION DETAIL https

D29006: Allow to copy or move selection to the other split view

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Actually sorry, I have a few more comments before I think this can land: INLINE COMMENTS > dolphintabwidget.h:196 > +/** Moves all selected items to the other view. */ > +

D29006: Allow to copy or move selection to the other split view

ngraham accepted this revision. ngraham added a comment. Thanks! REPOSITORY R318 Dolphin BRANCH arcpatch-D29006 REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, wai

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. In D29006#671172 , @ngraham wrote: > Nice feature. I would rename the menu items a bit: > "Move to inactive split view" > "Copy to inactive split view" > > No need to mention "the selection" because it's implicit

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 82880. aprcela added a comment. - Minor renaming of text and the according variables - Merge remote-tracking branch 'origin' into arcpatch-D29006 REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=82628&id=82880

D29006: Allow to copy or move selection to the other split view

ngraham added a comment. Nice feature. I would rename the menu items a bit: "Move to inactive split view" "Copy to inactive split view" No need to mention "the selection" because it's implicit. And I would mention that this is about the split view, or else when not using the split vie

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. In D29006#670660 , @meven wrote: > Rather than merging master into your branches, I would recommend you rebasing ;) Ok, thanks for the information! :) REPOSITORY R318 Dolphin BRANCH arcpatch-D29006 REVISION

D29006: Allow to copy or move selection to the other split view

meven accepted this revision. meven added a comment. This revision is now accepted and ready to land. Rather than merging master into your branches, I would recommend you rebasing ;) REPOSITORY R318 Dolphin BRANCH arcpatch-D29006 REVISION DETAIL https://phabricator.kde.org/D29006 To:

D29006: Allow to copy or move selection to the other split view

elvisangelaccio accepted this revision. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprcela

D29006: Allow to copy or move selection to the other split view

aprcela marked 2 inline comments as done. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, waitquietly, azyx, nikolaik, pberestov, iasensio, aprce

D29006: Allow to copy or move selection to the other split view

aprcela marked 3 inline comments as done. aprcela added inline comments. INLINE COMMENTS > elvisangelaccio wrote in dolphinview.h:377 > Let's just call it `destinationUrl`. This can be any URL, not necessarily the > URL of a split pane. Yeah, makes sense. Done REPOSITORY R318 Dolphin REVISI

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 82628. aprcela marked an inline comment as done. aprcela added a comment. - Renamed to destinationUrl REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=82609&id=82628 BRANCH arcpatch-D29006 REVISION DETAIL h

D29006: Allow to copy or move selection to the other split view

elvisangelaccio added inline comments. INLINE COMMENTS > dolphinview.h:377 > + */ > +void copySelectedItems(const KFileItemList &selection, const QUrl > &destinationPanelUrl); > + Let's just call it `destinationUrl`. This can be any URL, not necessarily the URL of a split pane. > dolp

D29006: Allow to copy or move selection to the other split view

aprcela marked an inline comment as done. aprcela added inline comments. INLINE COMMENTS > elvisangelaccio wrote in dolphinview.h:367-377 > Not anymore since d1a70c0b62 > ;) Thanks. I renamed those now :) REPOSITORY

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 82609. aprcela added a comment. - Merge branch 'master' of https://anongit.kde.org/dolphin into arcpatch-D29006 - Merge branch 'master' of https://anongit.kde.org/dolphin into arcpatch-D29006 - Rename to (copy|move)SelectedItems REPOSITORY R318 Dol

D29006: Allow to copy or move selection to the other split view

elvisangelaccio added inline comments. INLINE COMMENTS > aprcela wrote in dolphinview.h:367-377 > `copySelectedItems()` is already used to copy to clipboard. See L365. Not anymore since d1a70c0b62 ;) REPOSITORY R318

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. In D29006#667654 , @meven wrote: > In D29006#667588 , @dfaure wrote: > > > It sounds to me like you found bugs in some kioslaves. Fixing that is out of scope for this change

D29006: Allow to copy or move selection to the other split view

meven added a comment. In D29006#667588 , @dfaure wrote: > It sounds to me like you found bugs in some kioslaves. Fixing that is out of scope for this change request. Certainly a good idea to do that, but IMHO not a blocker for this to go in.

D29006: Allow to copy or move selection to the other split view

dfaure accepted this revision. dfaure added a comment. Good from a KIO point of view, but this needs approval from Dolphin people. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-do

D29006: Allow to copy or move selection to the other split view

dfaure added a comment. It sounds to me like you found bugs in some kioslaves. Fixing that is out of scope for this change request. Certainly a good idea to do that, but IMHO not a blocker for this to go in. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To:

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. @meven These seem to be 'special-cases' locations. The call to `capabilitiesDestination.isWritable()` : Does not work for these. An error message pops up when trying to move/copy here): remote:/ recentlyused:/ search:/ Does work as expected, follo

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81872. aprcela added a comment. - Rewrite so it's obvious that it is copying/moving to the inactive view REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=81854&id=81872 BRANCH arcpatch-D29006 REVISION DETAIL

D29006: Allow to copy or move selection to the other split view

aprcela edited the summary of this revision. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice

D29006: Allow to copy or move selection to the other split view

aprcela edited the summary of this revision. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81854. aprcela added a comment. - Merge branch 'master' of https://anongit.kde.org/dolphin into arcpatch-D29006 REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=81850&id=81854 BRANCH arcpatch-D29006 REVISION

D29006: Allow to copy or move selection to the other split view

aprcela marked an inline comment as done. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, g

D29006: Allow to copy or move selection to the other split view

aprcela marked an inline comment as done. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, g

D29006: Allow to copy or move selection to the other split view

aprcela marked an inline comment as done. aprcela added inline comments. INLINE COMMENTS > aprcela wrote in dolphinview.cpp:1222 > Ok. I'm gonna make a separate patch via phabricator. https://phabricator.kde.org/D29399 REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81850. aprcela marked an inline comment as done. aprcela added a comment. - Revert "Instead of append, change the value to a simplifiedUrlList" REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=81826&id=81850 BRA

D29006: Allow to copy or move selection to the other split view

aprcela marked an inline comment as done. aprcela added inline comments. INLINE COMMENTS > meven wrote in dolphinview.cpp:1222 > No your individual commits are supposed to get squashed upon merging. Ok. I'm gonna make a separate patch via phabricator. REPOSITORY R318 Dolphin REVISION DETAIL

D29006: Allow to copy or move selection to the other split view

meven added inline comments. INLINE COMMENTS > aprcela wrote in dolphinview.cpp:1222 > It's in the commit 5359352b0ef1. Is that enough? No your individual commits are supposed to get squashed upon merging. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprc

D29006: Allow to copy or move selection to the other split view

aprcela marked 2 inline comments as done. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, g

D29006: Allow to copy or move selection to the other split view

aprcela marked 2 inline comments as done. aprcela added a comment. @elvisangelaccio wrote: copySelectedItems() is already used to copy to clipboard. See L365. @meven I uploadaded the currently working code (to check if destination is writable for the copy function). But I can't get i

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81826. aprcela marked 2 inline comments as done. aprcela added a comment. - Add check if destination is writable for copy - Move copy/move logic to DolphinTabWidget - Set Shift+F5 /F6 instead of CTRL+F5

D29006: Allow to copy or move selection to the other split view

elvisangelaccio requested changes to this revision. elvisangelaccio added a comment. The feature looks reasonable to me. INLINE COMMENTS > dolphinmainwindow.cpp:670-684 > +void DolphinMainWindow::copyToOtherView() > +{ > +const DolphinTabPage* tabPage = m_tabWidget->currentTabPage(); > +

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. In D29006#662058 , @meven wrote: > Just a check to add. @meven See inline comment In D29006#662094 , @dfaure wrote: > Actually, wait, I vote against Ctrl+F5

D29006: Allow to copy or move selection to the other split view

dfaure requested changes to this revision. dfaure added a comment. Actually, wait, I vote against Ctrl+F5 /Ctrl+F6 because these shortcuts, by default, in Plasma, are bound to "Switch to Desktop 5" and "Switch to Desktop 6". Shift+F5

D29006: Allow to copy or move selection to the other split view

meven requested changes to this revision. meven added a comment. This revision now requires changes to proceed. I tested the patch, works nicely. Just a check to add. Drag'n drop is quite easy in the use case it covers, so wait for @ngraham and/or @elvisangelaccio feedback before mergin

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. In D29006#661964 , @dfaure wrote: > Looks good to me. > > Did you test copying and moving directories as well? Should work, but just to make sure. > > I'll let the Dolphin developers/maintainers decide on whether

D29006: Allow to copy or move selection to the other split view

dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Looks good to me. Did you test copying and moving directories as well? Should work, but just to make sure. I'll let the Dolphin developers/maintainers decide on whether the shortcu

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81765. aprcela added a comment. - Connect to copyingDone to update m_selectedUrls REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=81757&id=81765 BRANCH arcpatch-D29006 REVISION DETAIL https://phabricator.k

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81757. aprcela marked 2 inline comments as done. aprcela added a comment. - Instead of append, change the value to a simplifiedUrlList - Remove 'selected' from text REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006

D29006: Allow to copy or move selection to the other split view

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > dolphinview.cpp:1222 > if (!m_selectedUrls.isEmpty()) { > m_selectedUrls << KDirModel::simplifiedUrlList(m_selectedUrls); > } This line of

D29006: Allow to copy or move selection to the other split view

meven added inline comments. INLINE COMMENTS > dolphinmainwindow.cpp:1414 > paste->setIconText(i18nc("@action:inmenu Edit", "Paste")); > -paste->setWhatsThis(xi18nc("@info:whatsthis paste", "This copies the > items from " > +paste->setWhatsThis(xi18nc("@info:whatsthis paste", "This

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81708. aprcela marked 2 inline comments as done. aprcela added a comment. - Rename slotPasteJobResult to more generic slotJobResult - Added error check when copying to other view REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. Done :) REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasensio, aprcela, fprice, gennad

D29006: Allow to copy or move selection to the other split view

dfaure added inline comments. INLINE COMMENTS > dolphinview.cpp:697 > +{ > +KIO::CopyJob* job = KIO::copy(selection.urlList(), destinationPanelUrl, > KIO::DefaultFlags); > +KJobWidgets::setWindow(job, this); Why is there no error handling for the copy job? It can fail too. (Try having /

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. @dfaure is it good now? Anyone found anything else to fix and/or improve? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-de

D29006: Allow to copy or move selection to the other split view

yurchor added a comment. Thanks for updating the docbook. +1 REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D29006 To: aprcela, #dolphin, elvisangelaccio, ngraham, meven, dfaure Cc: yurchor, kde-doc-english, dfaure, meven, kfm-devel, azyx, nikolaik, pberestov, iasen

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81573. aprcela marked 7 inline comments as done. aprcela added a comment. - Update docbook REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29006?vs=81509&id=81573 BRANCH arcpatch-D29006 REVISION DETAIL https://phab

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. In D29006#660096 , @dfaure wrote: > You can compare with what happens when doing the same invalid move using Cut / Paste for instance. > > The job gives the error "A folder cannot be moved into itself" and dolphin di

D29006: Allow to copy or move selection to the other split view

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > dolphinmainwindow.cpp:1973 > +copyToOtherViewAction->setEnabled(false); > +moveToOtherViewAction->setEnabled(capabilities.supportsMoving()); >

D29006: Allow to copy or move selection to the other split view

dfaure added a comment. You can compare with what happens when doing the same invalid move using Cut / Paste for instance. The job gives the error "A folder cannot be moved into itself" and dolphin displays this in the view. You want to connect to the job's result() signal and emit

D29006: Allow to copy or move selection to the other split view

aprcela updated this revision to Diff 81509. aprcela added a comment. Herald added a project: Documentation. Herald added a subscriber: kde-doc-english. - Make moving only available if file is movable. - Update docbook REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricato

D29006: Allow to copy or move selection to the other split view

aprcela added a comment. One more fix and update. Is it ok that nothing happens if a user wants to move a folder into one of it's subfolders? If dolphin is launched from bash, one can see `"Could not rename file /tmp/subfolder copy."` if one tries to move `/tmp/subfolder` to `/tmp/subfol