Changeset View
Standalone View
src/widgets/dropjob.cpp
Show All 31 Lines | |||||
32 | #include <KDesktopFile> | 32 | #include <KDesktopFile> | ||
33 | #include <KIO/CopyJob> | 33 | #include <KIO/CopyJob> | ||
34 | #include <KIO/DndPopupMenuPlugin> | 34 | #include <KIO/DndPopupMenuPlugin> | ||
35 | #include <KIO/FileUndoManager> | 35 | #include <KIO/FileUndoManager> | ||
36 | #include <KFileItem> | 36 | #include <KFileItem> | ||
37 | #include <KFileItemListProperties> | 37 | #include <KFileItemListProperties> | ||
38 | #include <KJobWidgets> | 38 | #include <KJobWidgets> | ||
39 | #include <KLocalizedString> | 39 | #include <KLocalizedString> | ||
40 | #include <KMountPoint> | ||||
40 | #include <KPluginMetaData> | 41 | #include <KPluginMetaData> | ||
41 | #include <KPluginLoader> | 42 | #include <KPluginLoader> | ||
42 | #include <KProtocolManager> | 43 | #include <KProtocolManager> | ||
43 | #include <KUrlMimeData> | 44 | #include <KUrlMimeData> | ||
44 | #include <KRun> | 45 | #include <KRun> | ||
45 | #include <KService> | 46 | #include <KService> | ||
47 | #include <KSharedConfig> | ||||
46 | 48 | | |||
47 | #include <QDropEvent> | 49 | #include <QDropEvent> | ||
48 | #include <QFileInfo> | 50 | #include <QFileInfo> | ||
49 | #include <QMenu> | 51 | #include <QMenu> | ||
50 | #include <QMimeData> | 52 | #include <QMimeData> | ||
51 | #include <QProcess> | 53 | #include <QProcess> | ||
52 | #include <QTimer> | 54 | #include <QTimer> | ||
53 | 55 | | |||
▲ Show 20 Lines • Show All 84 Lines • ▼ Show 20 Line(s) | 84 | public: | |||
138 | void fillPopupMenu(KIO::DropMenu *popup); | 140 | void fillPopupMenu(KIO::DropMenu *popup); | ||
139 | void addPluginActions(KIO::DropMenu *popup, const KFileItemListProperties &itemProps); | 141 | void addPluginActions(KIO::DropMenu *popup, const KFileItemListProperties &itemProps); | ||
140 | void doCopyToDirectory(); | 142 | void doCopyToDirectory(); | ||
141 | 143 | | |||
142 | const QMimeData *m_mimeData; | 144 | const QMimeData *m_mimeData; | ||
143 | const QList<QUrl> m_urls; | 145 | const QList<QUrl> m_urls; | ||
144 | QMap<QString, QString> m_metaData; | 146 | QMap<QString, QString> m_metaData; | ||
145 | Qt::DropAction m_dropAction; | 147 | Qt::DropAction m_dropAction; | ||
148 | Qt::DropActions m_possibleActions; | ||||
146 | QPoint m_relativePos; | 149 | QPoint m_relativePos; | ||
147 | Qt::KeyboardModifiers m_keyboardModifiers; | 150 | Qt::KeyboardModifiers m_keyboardModifiers; | ||
151 | KFileItemListProperties m_itemProps; | ||||
meven: I think you don't need a member here, a local variable should suffice. | |||||
148 | QUrl m_destUrl; | 152 | QUrl m_destUrl; | ||
149 | KFileItem m_destItem; // null for remote URLs not found in the dirlister cache | 153 | KFileItem m_destItem; // null for remote URLs not found in the dirlister cache | ||
150 | const JobFlags m_flags; | 154 | const JobFlags m_flags; | ||
151 | const DropJobFlags m_dropjobFlags; | 155 | const DropJobFlags m_dropjobFlags; | ||
152 | QList<QAction *> m_appActions; | 156 | QList<QAction *> m_appActions; | ||
153 | QList<QAction *> m_pluginActions; | 157 | QList<QAction *> m_pluginActions; | ||
154 | bool m_triggered; // Tracks whether an action has been triggered in the popup menu. | 158 | bool m_triggered; // Tracks whether an action has been triggered in the popup menu. | ||
155 | QSet<KIO::DropMenu *> m_menus; | 159 | QSet<KIO::DropMenu *> m_menus; | ||
▲ Show 20 Lines • Show All 106 Lines • ▼ Show 20 Line(s) | 263 | { | |||
262 | if (!KProtocolManager::supportsWriting(m_destUrl)) { | 266 | if (!KProtocolManager::supportsWriting(m_destUrl)) { | ||
263 | return KIO::ERR_CANNOT_WRITE; | 267 | return KIO::ERR_CANNOT_WRITE; | ||
264 | } | 268 | } | ||
265 | 269 | | |||
266 | if (!m_destItem.isNull() && !m_destItem.isWritable() && (m_flags & KIO::NoPrivilegeExecution)) { | 270 | if (!m_destItem.isNull() && !m_destItem.isWritable() && (m_flags & KIO::NoPrivilegeExecution)) { | ||
267 | return KIO::ERR_WRITE_ACCESS_DENIED; | 271 | return KIO::ERR_WRITE_ACCESS_DENIED; | ||
268 | } | 272 | } | ||
269 | 273 | | |||
274 | // Check what the source can do | ||||
275 | // TODO: Determining the mimetype of the source URLs is difficult for remote URLs, | ||||
276 | // we would need to KIO::stat each URL in turn, asynchronously.... | ||||
277 | KFileItemList fileItems; | ||||
278 | fileItems.reserve(m_urls.size()); | ||||
279 | | ||||
270 | bool allItemsAreFromTrash = true; | 280 | bool allItemsAreFromTrash = true; | ||
281 | bool allItemsAreLocal = true; | ||||
282 | bool allItemsAreSameDevice = true; | ||||
271 | bool containsTrashRoot = false; | 283 | bool containsTrashRoot = false; | ||
284 | bool equalDestination = true; | ||||
285 | | ||||
286 | KMountPoint::List mountPoints = KMountPoint::currentMountPoints(); | ||||
Why QStorageInfo? We're in kio. There's a KMountPoint which is similar I suspect this is a recursive list up the tree resolving symlinks. This will mean blocking stat calls, so this somewhat undermines a lot of the recent work in that field. davidedmundson: Why QStorageInfo? We're in kio. There's a KMountPoint which is similar
I suspect this is a… | |||||
I don't understand this. Could you please explain a bit more? trmdi: > I suspect this is a recursive list up the tree resolving symlinks. This will mean blocking… | |||||
287 | const QString &destDevice = mountPoints.findByPath(m_destUrl.path())->mountedFrom(); | ||||
If destination is a symlink on the same partition but that symlink points to another partition what happens? I haven't checked myself, but if we are doing this I need us to be super super sure. davidedmundson: If destination is a symlink on the same partition but that symlink points to another partition… | |||||
trmdi: They are considered as different partitions, I checked. | |||||
288 | | ||||
272 | for (const QUrl &url : m_urls) { | 289 | for (const QUrl &url : m_urls) { | ||
273 | const bool local = url.isLocalFile(); | 290 | const bool local = url.isLocalFile(); | ||
274 | if (!local /*optimization*/ && url.scheme() == QLatin1String("trash")) { | 291 | if (!local /*optimization*/ && url.scheme() == QLatin1String("trash")) { | ||
275 | if (url.path().isEmpty() || url.path() == QLatin1String("/")) { | 292 | if (url.path().isEmpty() || url.path() == QLatin1String("/")) { | ||
276 | containsTrashRoot = true; | 293 | containsTrashRoot = true; | ||
277 | } | 294 | } | ||
278 | } else { | 295 | } else { | ||
279 | allItemsAreFromTrash = false; | 296 | allItemsAreFromTrash = false; | ||
297 | | ||||
298 | if (!local && allItemsAreLocal) { | ||||
299 | allItemsAreLocal = false; | ||||
300 | } | ||||
301 | } | ||||
302 | | ||||
meven: You could add `equalDestination &&` here to save further checking. | |||||
303 | if (equalDestination && !m_destUrl.matches(url.adjusted(QUrl::RemoveFilename), QUrl::StripTrailingSlash)) { | ||||
304 | equalDestination = false; | ||||
280 | } | 305 | } | ||
This will trigger when both storage devices are invalid, which isn't what we want. davidedmundson: This will trigger when both storage devices are invalid, which isn't what we want. | |||||
I don't understand this. How could devices be invalid if we are dropping valid urls to a valid destUrl? trmdi: I don't understand this. How could devices be invalid if we are dropping valid urls to a valid… | |||||
306 | | ||||
307 | if (allItemsAreSameDevice) { | ||||
308 | const QString &sourceDevice = mountPoints.findByPath(url.path())->mountedFrom(); | ||||
309 | if (sourceDevice != destDevice && !KFileItem(url).isLink()) { | ||||
You can create a KFileItem(url) before, so that you can reuse it line 314, instead of building one again. meven: You can create a `KFileItem(url)` before, so that you can reuse it line 314, instead of… | |||||
310 | allItemsAreSameDevice = false; | ||||
311 | } | ||||
312 | } | ||||
313 | | ||||
314 | fileItems.append(KFileItem(url)); | ||||
315 | | ||||
281 | if (url.matches(m_destUrl, QUrl::StripTrailingSlash)) { | 316 | if (url.matches(m_destUrl, QUrl::StripTrailingSlash)) { | ||
282 | return KIO::ERR_DROP_ON_ITSELF; | 317 | return KIO::ERR_DROP_ON_ITSELF; | ||
283 | } | 318 | } | ||
284 | } | 319 | } | ||
320 | m_itemProps.setItems(fileItems); | ||||
321 | | ||||
322 | m_possibleActions = Qt::LinkAction; | ||||
323 | const bool sReading = m_itemProps.supportsReading(); | ||||
324 | const bool sDeleting = m_itemProps.supportsDeleting(); | ||||
325 | const bool sMoving = m_itemProps.supportsMoving(); | ||||
326 | | ||||
327 | if (sReading) { | ||||
328 | m_possibleActions |= Qt::CopyAction; | ||||
329 | } | ||||
330 | | ||||
331 | if (sMoving || (sReading && sDeleting)) { | ||||
332 | if (!equalDestination) { | ||||
333 | m_possibleActions |= Qt::MoveAction; | ||||
334 | } | ||||
335 | } | ||||
285 | 336 | | |||
286 | const bool trashing = m_destUrl.scheme() == QLatin1String("trash"); | 337 | const bool trashing = m_destUrl.scheme() == QLatin1String("trash"); | ||
287 | if (trashing) { | 338 | if (trashing) { | ||
288 | if (allItemsAreFromTrash) { | 339 | if (allItemsAreFromTrash) { | ||
289 | qCDebug(KIO_WIDGETS) << "Dropping items from trash to trash"; | 340 | qCDebug(KIO_WIDGETS) << "Dropping items from trash to trash"; | ||
290 | return KIO::ERR_DROP_ON_ITSELF; | 341 | return KIO::ERR_DROP_ON_ITSELF; | ||
291 | } | 342 | } | ||
292 | m_dropAction = Qt::MoveAction; | 343 | m_dropAction = Qt::MoveAction; | ||
Show All 12 Lines | 354 | if (containsTrashRoot) { | |||
305 | m_dropAction = Qt::LinkAction; | 356 | m_dropAction = Qt::LinkAction; | ||
306 | return KJob::NoError; // ok | 357 | return KJob::NoError; // ok | ||
307 | } | 358 | } | ||
308 | if (allItemsAreFromTrash) { | 359 | if (allItemsAreFromTrash) { | ||
309 | // No point in asking copy/move/link when using dragging from the trash, just move the file out. | 360 | // No point in asking copy/move/link when using dragging from the trash, just move the file out. | ||
310 | m_dropAction = Qt::MoveAction; | 361 | m_dropAction = Qt::MoveAction; | ||
311 | return KJob::NoError; // ok | 362 | return KJob::NoError; // ok | ||
312 | } | 363 | } | ||
364 | | ||||
365 | // if the default behavior has been changed to MoveAction, read from kdeglobals | ||||
366 | const KConfigGroup g = KConfigGroup(KSharedConfig::openConfig(), QStringLiteral("KDE")); | ||||
there's no need to reopen kdelgobals Use KSharedConfig::openConfig() and due to the magic of cascading it will include kdeglobals davidedmundson: there's no need to reopen kdelgobals
Use KSharedConfig::openConfig()
and due to the magic of… | |||||
367 | if (g.readEntry("dndToMove", false) && (m_possibleActions & Qt::MoveAction) && allItemsAreLocal && allItemsAreSameDevice) { | ||||
368 | if (m_keyboardModifiers == Qt::NoModifier) { | ||||
369 | m_dropAction = Qt::MoveAction; | ||||
370 | return KJob::NoError; // ok | ||||
371 | } else if (m_keyboardModifiers == Qt::ShiftModifier) { | ||||
372 | // the user requests to show the menu | ||||
373 | return KIO::ERR_UNKNOWN; | ||||
374 | } | ||||
375 | } | ||||
376 | | ||||
313 | if (m_keyboardModifiers & (Qt::ControlModifier | Qt::ShiftModifier | Qt::AltModifier)) { | 377 | if (m_keyboardModifiers & (Qt::ControlModifier | Qt::ShiftModifier | Qt::AltModifier)) { | ||
314 | // Qt determined m_dropAction from the modifiers already | 378 | // Qt determined m_dropAction from the modifiers already | ||
315 | return KJob::NoError; // ok | 379 | return KJob::NoError; // ok | ||
316 | } | 380 | } | ||
317 | 381 | | |||
318 | // We need to ask the user with a popup menu. Let the caller know. | 382 | // We need to ask the user with a popup menu. Let the caller know. | ||
319 | return KIO::ERR_UNKNOWN; | 383 | return KIO::ERR_UNKNOWN; | ||
320 | } | 384 | } | ||
maybe we should call it something more descriptive, like "dndMoveByDefault"? ngraham: maybe we should call it something more descriptive, like "dndMoveByDefault"? | |||||
321 | 385 | | |||
322 | void DropJobPrivate::fillPopupMenu(KIO::DropMenu *popup) | 386 | void DropJobPrivate::fillPopupMenu(KIO::DropMenu *popup) | ||
323 | { | 387 | { | ||
324 | Q_Q(DropJob); | 388 | Q_Q(DropJob); | ||
325 | 389 | | |||
326 | // Check what the source can do | 390 | emit q->popupMenuAboutToShow(m_itemProps); | ||
327 | // TODO: Determining the mimetype of the source URLs is difficult for remote URLs, | | |||
328 | // we would need to KIO::stat each URL in turn, asynchronously.... | | |||
329 | KFileItemList fileItems; | | |||
330 | fileItems.reserve(m_urls.size()); | | |||
331 | for (const QUrl &url : m_urls) { | | |||
332 | fileItems.append(KFileItem(url)); | | |||
333 | } | | |||
334 | const KFileItemListProperties itemProps(fileItems); | | |||
335 | | ||||
336 | emit q->popupMenuAboutToShow(itemProps); | | |||
337 | | ||||
338 | const bool sReading = itemProps.supportsReading(); | | |||
339 | const bool sDeleting = itemProps.supportsDeleting(); | | |||
340 | const bool sMoving = itemProps.supportsMoving(); | | |||
341 | 391 | | |||
342 | QString seq = QKeySequence(Qt::ShiftModifier).toString(); | 392 | QString seq = QKeySequence(Qt::ShiftModifier).toString(); | ||
343 | Q_ASSERT(seq.endsWith(QLatin1Char('+'))); | 393 | Q_ASSERT(seq.endsWith(QLatin1Char('+'))); | ||
344 | seq.chop(1); // chop superfluous '+' | 394 | seq.chop(1); // chop superfluous '+' | ||
345 | QAction* popupMoveAction = new QAction(i18n("&Move Here") + QLatin1Char('\t') + seq, popup); | 395 | QAction* popupMoveAction = new QAction(i18n("&Move Here") + QLatin1Char('\t') + seq, popup); | ||
346 | popupMoveAction->setIcon(QIcon::fromTheme(QStringLiteral("go-jump"))); | 396 | popupMoveAction->setIcon(QIcon::fromTheme(QStringLiteral("go-jump"))); | ||
347 | popupMoveAction->setData(QVariant::fromValue(Qt::MoveAction)); | 397 | popupMoveAction->setData(QVariant::fromValue(Qt::MoveAction)); | ||
348 | seq = QKeySequence(Qt::ControlModifier).toString(); | 398 | seq = QKeySequence(Qt::ControlModifier).toString(); | ||
349 | Q_ASSERT(seq.endsWith(QLatin1Char('+'))); | 399 | Q_ASSERT(seq.endsWith(QLatin1Char('+'))); | ||
350 | seq.chop(1); | 400 | seq.chop(1); | ||
351 | QAction* popupCopyAction = new QAction(i18n("&Copy Here") + QLatin1Char('\t') + seq, popup); | 401 | QAction* popupCopyAction = new QAction(i18n("&Copy Here") + QLatin1Char('\t') + seq, popup); | ||
352 | popupCopyAction->setIcon(QIcon::fromTheme(QStringLiteral("edit-copy"))); | 402 | popupCopyAction->setIcon(QIcon::fromTheme(QStringLiteral("edit-copy"))); | ||
353 | popupCopyAction->setData(QVariant::fromValue(Qt::CopyAction)); | 403 | popupCopyAction->setData(QVariant::fromValue(Qt::CopyAction)); | ||
354 | seq = QKeySequence(Qt::ControlModifier + Qt::ShiftModifier).toString(); | 404 | seq = QKeySequence(Qt::ControlModifier + Qt::ShiftModifier).toString(); | ||
355 | Q_ASSERT(seq.endsWith(QLatin1Char('+'))); | 405 | Q_ASSERT(seq.endsWith(QLatin1Char('+'))); | ||
356 | seq.chop(1); | 406 | seq.chop(1); | ||
357 | QAction* popupLinkAction = new QAction(i18n("&Link Here") + QLatin1Char('\t') + seq, popup); | 407 | QAction* popupLinkAction = new QAction(i18n("&Link Here") + QLatin1Char('\t') + seq, popup); | ||
358 | popupLinkAction->setIcon(QIcon::fromTheme(QStringLiteral("edit-link"))); | 408 | popupLinkAction->setIcon(QIcon::fromTheme(QStringLiteral("edit-link"))); | ||
359 | popupLinkAction->setData(QVariant::fromValue(Qt::LinkAction)); | 409 | popupLinkAction->setData(QVariant::fromValue(Qt::LinkAction)); | ||
360 | 410 | | |||
361 | if (sMoving || (sReading && sDeleting)) { | 411 | if (m_possibleActions & Qt::MoveAction) { | ||
362 | bool equalDestination = true; | | |||
363 | for (const QUrl &src : m_urls) { | | |||
364 | if (!m_destUrl.matches(src.adjusted(QUrl::RemoveFilename), QUrl::StripTrailingSlash)) { | | |||
365 | equalDestination = false; | | |||
366 | break; | | |||
367 | } | | |||
368 | } | | |||
369 | | ||||
370 | if (!equalDestination) { | | |||
371 | popup->addAction(popupMoveAction); | 412 | popup->addAction(popupMoveAction); | ||
372 | } | 413 | } | ||
373 | } | | |||
374 | 414 | | |||
375 | if (sReading) { | 415 | if (m_possibleActions & Qt::CopyAction) { | ||
376 | popup->addAction(popupCopyAction); | 416 | popup->addAction(popupCopyAction); | ||
377 | } | 417 | } | ||
378 | 418 | | |||
379 | popup->addAction(popupLinkAction); | 419 | popup->addAction(popupLinkAction); | ||
380 | 420 | | |||
381 | addPluginActions(popup, itemProps); | 421 | addPluginActions(popup, m_itemProps); | ||
382 | 422 | | |||
383 | popup->addSeparator(); | 423 | popup->addSeparator(); | ||
384 | } | 424 | } | ||
385 | 425 | | |||
386 | void DropJobPrivate::addPluginActions(KIO::DropMenu *popup, const KFileItemListProperties &itemProps) | 426 | void DropJobPrivate::addPluginActions(KIO::DropMenu *popup, const KFileItemListProperties &itemProps) | ||
387 | { | 427 | { | ||
388 | const QVector<KPluginMetaData> plugin_offers = KPluginLoader::findPlugins(QStringLiteral("kf5/kio_dnd")); | 428 | const QVector<KPluginMetaData> plugin_offers = KPluginLoader::findPlugins(QStringLiteral("kf5/kio_dnd")); | ||
389 | for (const KPluginMetaData &service : plugin_offers) { | 429 | for (const KPluginMetaData &service : plugin_offers) { | ||
▲ Show 20 Lines • Show All 212 Lines • Show Last 20 Lines |
I think you don't need a member here, a local variable should suffice.