Differential D8208 Diff 32889 src/views/dolphinview.cpp

Changeset View

Standalone View

View Options

src/views/dolphinview.cpp

Show First 20 Lines • Show All 697 Lines • ▼ Show 20 Line(s)
698void DolphinView::pasteIntoFolder() 698​void DolphinView::pasteIntoFolder()
699{ 699​{
700 const KFileItemList items = selectedItems(); 700​ const KFileItemList items = selectedItems();
701 if ((items.count() == 1) && items.first().isDir()) { 701​ if ((items.count() == 1) && items.first().isDir()) {
702 pasteToUrl(items.first().url()); 702​ pasteToUrl(items.first().url());
703 } 703​ }
704} 704​}
705 705​
706​void DolphinView::duplicateSelectedItems()
707​{
708​ const KFileItemList itemList = selectedItems();
709​ if (itemList.isEmpty()) {
710​ return;
711​ }
712​
713​ QUrl originalURL;
714​ QFileInfo originalFile;
715​ QString originalPath;
716​ QString suffix;
717​ QString suffixString;
718​ QUrl duplicateURL;
pinoUnsubmitted
Done

using QFileInfo will break if the url is not a local file

pino: using QFileInfo will break if the url is not a local file
719​ QString duplicatePath;
720​ QList<QUrl> newSelection;
dfaureUnsubmitted
Done

This will behave wrong on /home/dfaure/Documents/dfaure
(it will remove the first dfaure instead of the second)
Substring search done by remove() is both dangerous and slow.

You could just do

const QString fileLocation = originalURL.path(QUrl::RemoveFilename);

(I'd call it directoryPath btw, in QUrl terms it's a path)

dfaure: This will behave wrong on /home/dfaure/Documents/dfaure (it will remove the first dfaure…
721​
fvogtUnsubmitted
Done

Note that this returns a null QString if the extension isn't known and also doesn't preserve case.

So currently this would happen:

"test.foo" -> "test.foo"/ ""` -> "test.foo copy"
"test.GIF" -> "test"/ "gif"` -> "test.GIF copy.gif"

fvogt: Note that this returns a null `QString` if the extension isn't known and also doesn't preserve…
722​ // If multiple items are selected, duplicate them all
723​ for (const auto &item) {
724​ originalURL = item.url();
725​ duplicateURL = originalURL;
dfaureUnsubmitted
Done

move it out of the loop

dfaure: move it out of the loop
726​ originalFile = QFileInfo(originalURL.path());
727​
elvisangelaccioUnsubmitted
Done

Please add context for translators, explaining that %1 is a path.

elvisangelaccio: Please add context for translators, explaining that %1 is a path.
728​ // We extract the file and suffix separately so we can append "copy"
729​ // to the end of the file name, but before the suffix, if there is one
730​ originalPath = originalFile.path() + "/" + originalFile.baseName();
731​ suffix = originalFile.completeSuffix();
elvisangelaccioUnsubmitted
Done

QLatin1String if you want to concatenate, QStringLiteral(".%1").arg(extension) otherwise.

elvisangelaccio: `QLatin1String` if you want to concatenate, `QStringLiteral(".%1").arg(extension)` otherwise.
732​
dfaureUnsubmitted
Done

path(), not toString().

Or if you really want a full URL in there then call it directoryURL. It means reparsing the whole URL though, I thought we'd just copy the URL and change the path.

dfaure: path(), not toString(). Or if you really want a full URL in there then call it directoryURL.
733​ // Don't add an unnecessary dot for directories or files with no suffix
734​ if (suffix.isEmpty()) {
735​ suffixString.clear();
736​ } else {
dfaureUnsubmitted
Done

It would be simpler to keep the dot here (then both this line and the previous one become simpler).

dfaure: It would be simpler to keep the dot here (then both this line and the previous one become…
elvisangelaccioUnsubmitted
Done

I'd call this variable originalExtension.

elvisangelaccio: I'd call this variable `originalExtension`.
737​ suffixString = "." + suffix;
dfaureUnsubmitted
Done

Why not just put the "." in the format string here?

dfaure: Why not just put the "." in the format string here?
ngrahamAuthorUnsubmitted
Done

Because then originalFileName.chopped(extension.size() includes the dot from the extension because extension doesn't already include it.

ngraham: Because then `originalFileName.chopped(extension.size()` includes the dot from the extension…
738​ }
739​
740​ if (originalPath.endsWith(i18n(" copy"))) {
741​ duplicatePath = originalPath + i18n(" 1") + suffixString;
broulikUnsubmitted
Done

You might want to look into KIO::suggestName or what it's called but I think this might not be completely suitable for this usecase here

broulik: You might want to look into `KIO::suggestName` or what it's called but I think this might not…
742​ } else {
743​ duplicatePath = originalPath + i18n(" copy") + suffixString;
744​ }
745​ duplicateURL.setPath(duplicatePath);
746​
747​ KIO::CopyJob* job = KIO::copyAs(originalURL, duplicateURL, KIO::HideProgressInfo);
748​ KJobWidgets::setWindow(job, this);
749​
750​ if (job) {
elvisangelaccioUnsubmitted
Done

What's this needed for?

elvisangelaccio: What's this needed for?
ngrahamAuthorUnsubmitted
Done

To make sure the new file(s) get selected after creation.

ngraham: To make sure the new file(s) get selected after creation.
751​ newSelection << duplicateURL;
752​ KIO::FileUndoManager::self()->recordCopyJob(job);
753​ }
elvisangelaccioUnsubmitted
Done

What's this connection needed for?

elvisangelaccio: What's this connection needed for?
754​ }
755​
756​ forceUrlsSelection(newSelection.first(), newSelection);
elvisangelaccioUnsubmitted
Not Done

Since this doesn't work, I'd just remove it.

(I still think that we don't need to trigger renaming in the first place).

elvisangelaccio: Since this doesn't work, I'd just remove it. (I still think that we don't need to trigger…
757​ emitSelectionChangedSignal();
758​
759​ connect(m_view, &DolphinItemListView::roleEditingFinished,
760​ this, &DolphinView::slotRoleEditingFinished);
761​}
762​
706void DolphinView::stopLoading() 763​void DolphinView::stopLoading()
707{ 764​{
708 m_model->cancelDirectoryLoading(); 765​ m_model->cancelDirectoryLoading();
709} 766​}
710 767​
711void DolphinView::updatePalette() 768​void DolphinView::updatePalette()
712{ 769​{
713 QColor color = KColorScheme(QPalette::Active, KColorScheme::View).background().color(); 770​ QColor color = KColorScheme(QPalette::Active, KColorScheme::View).background().color();
▲ Show 20 Lines • Show All 1097 Lines • Show Last 20 Lines