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

Elvis Angelaccio noreply at phabricator.kde.org
Sun May 3 18:41:08 BST 2020


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();
> +    if (!tabPage->splitViewEnabled() || m_activeViewContainer->view()->selectedItems().isEmpty()) {
> +        return;
> +    }
> +

Can you try to add a `DolphinTabWidget::copyToOtherView()` function and move this logic there?

> dolphinmainwindow.cpp:686-700
> +void DolphinMainWindow::moveToOtherView()
> +{
> +    const DolphinTabPage* tabPage = m_tabWidget->currentTabPage();
> +    if (!tabPage->splitViewEnabled() || m_activeViewContainer->view()->selectedItems().isEmpty()) {
> +        return;
> +    }
> +

Can you try to add a `DolphinTabWidget::moveToOtherView()` function and move this logic there?

> dfaure wrote in dolphinview.cpp:1222
> This line of code (from me, it turns out) is bogus, it should say `=` instead of `<<`.
> We want to replace the list with the simplified list, not concatenate the two.
> 
> And m_selectedUrls needs to contain the new URLs, just like we do when pasting.
> 
> The solution I have in mind for that is to connect to CopyJob::slotCopyingDone and grab the "to" argument.
> 
> [discussed on IRC, but writing here for the record, and in case someone wonders why this << gets modified]

I see.

@aprcela Can you add this information in the commit message? Otherwise it will get lost.

> dolphinview.h:367-377
> +    /**
> +     * Copies all selected items to the other view.
> +     * Only available in Split View.
> +     */
> +    void copySelectedItemsToOtherSplitView(const KFileItemList &selection, const QUrl &destinationPanelUrl);
> +
> +    /**

Nitpick: `DolphinView` doesn't know what a "split view" is. I'd rather call these methods `copySelectedItems()` and `moveSelectedItems()`, since that's what they are actually doing.

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, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, rdieter, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200503/add265de/attachment.htm>


More information about the kfm-devel mailing list