Differential D10804 Diff 28173 src/trash/dolphintrash.cpp

Changeset View

Standalone View

View Options

src/trash/dolphintrash.cpp

  • This file was added.
1​/***************************************************************************
2​ * Copyright (C) 2018 by Roman Inflianskas <infroma@gmail.com> *
3​ * *
4​ * This program is free software; you can redistribute it and/or modify *
5​ * it under the terms of the GNU General Public License as published by *
6​ * the Free Software Foundation; either version 2 of the License, or *
7​ * (at your option) any later version. *
8​ * *
9​ * This program is distributed in the hope that it will be useful, *
10​ * but WITHOUT ANY WARRANTY; without even the implied warranty of *
11​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
12​ * GNU General Public License for more details. *
13​ * *
14​ * You should have received a copy of the GNU General Public License *
15​ * along with this program; if not, write to the *
16​ * Free Software Foundation, Inc., *
17​ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA *
18​ ***************************************************************************/
19​
20​#include "dolphintrash.h"
21​
22​#include <KIO/EmptyTrashJob>
23​#include <KIO/JobUiDelegate>
24​#include <KJobWidgets>
25​#include <QList>
26​#include <QUrl>
27​#include <QWidget>
28​#include <KNotification>
29​#include <KConfig>
30​#include <KConfigGroup>
31​
32​bool emptyTrash(QWidget *window)
33​{
34​ KIO::JobUiDelegate uiDelegate;
35​ uiDelegate.setWindow(window);
36​ bool confirmed = uiDelegate.askDeleteConfirmation(QList<QUrl>(), KIO::JobUiDelegate::EmptyTrash, KIO::JobUiDelegate::DefaultConfirmation);
37​ if (confirmed) {
38​ KIO::Job* job = KIO::emptyTrash();
39​ KJobWidgets::setWindow(job, window);
40​ job->uiDelegate()->setAutoErrorHandlingEnabled(true);
41​ QObject::connect(job, &KIO::Job::result, [](){
42​ KNotification::event(QStringLiteral("Trash: emptied"), QString(), QPixmap(), nullptr, KNotification::DefaultEvent);
43​ });
44​ }
45​ return confirmed;
46​}
47​
48​bool isTrashEmpty()
49​{
50​ KConfig trashConfig(QStringLiteral("trashrc"), KConfig::SimpleConfig);
markgUnsubmitted
Done

I see this was there before (in a different location), but i just don't get this part. "trashrc" is only used here (I've searched through the code) and it looks like nothing sets it hence the default is used which is true which in turn indicates the trash is empty.

Why?
This just doesn't make sense to me.

Or.. i can't search and it is used elsewhere, but then i didn't find it.

markg: I see this was there before (in a different location), but i just don't get this part.
rkflxUnsubmitted
Done

@markg https://lxr.kde.org/source/frameworks/kio/src/core/kfileitem.cpp#0883:

// We need to find if the trash is empty, preferably without using a KIO job.
// So instead kio_trash leaves an entry in its config file for us.

Cannot comment on whether that's only applicable for KIO or Dolphin too.

rkflx: @markg https://lxr.kde.org/source/frameworks/kio/src/core/kfileitem.cpp#0883: // We need to…
markgUnsubmitted
Done

Ahh, that makes sense.
If dolphin only needed the icon it wouldn't need to look at that file, but it also needs to know the state (empty or full) so it looks at the file.

Thank you for the pointer! No wonder i couldn't find it in Dolphin, it wasn't there ;)

markg: Ahh, that makes sense. If dolphin only needed the icon it wouldn't need to look at that file…
51​ return (trashConfig.group("Status").readEntry("Empty", true));
markgUnsubmitted
Done

You probably don't want to do that. As a mere setting would decide if the trash is empty or not.
You can do this in a few different ways.

  1. Look at placesitem.cpp at the "m_trashDirLister" how that is used to update the trash icon in the places menu with whatever the trash state is.

Spoiler:

void PlacesItem::onTrashDirListerCompleted()
{
    Q_ASSERT(url().scheme() == QLatin1String("trash"));

    const bool isTrashEmpty = m_trashDirLister->items().isEmpty();
    setIcon(isTrashEmpty ? QStringLiteral("user-trash") : QStringLiteral("user-trash-full"));
}

Also, you already have the information if the trash is empty. You have the view status information (the thingy below the view that says "x Folders, X files". Follow that to figure out how it fills that information which would also give you the trash state. This would be the way to go imho.

markg: You probably don't want to do that. As a mere setting would decide if the trash is empty or not.
rkflxUnsubmitted
Done

@rominf Did you look at Mark's comment already?

rkflx: @rominf Did you look at Mark's comment already?
rominfAuthorUnsubmitted
Done

Look at placesitem.cpp at the "m_trashDirLister" how that is used to update the trash icon in the places menu with whatever the trash state is.

This code doesn't work well. See https://bugs.kde.org/show_bug.cgi?id=252483

You probably don't want to do that. As a mere setting would decide if the trash is empty or not.

This way is not my idea. I asked David Faure, the KIO maintainer. He said that this way is used everywhere (it's used in Dolphin too). He also said that because of that it's a nice idea to put it to KIO.

rominf: > Look at placesitem.cpp at the "m_trashDirLister" how that is used to update the trash icon in…
52​}
markgUnsubmitted
Done

You now have a memory leak. The trashDirLister is never deleted.

markg: You now have a memory leak. The trashDirLister is never deleted.
markgUnsubmitted
Done

missing space after the comma.

markg: missing space after the comma.
markgUnsubmitted
Done

This - as i said in my last message - smells like a bug. If that is required then KIO likely has a bug, Don't fix this here, hunt for the issue in KIO please.

markg: This - as i said in my last message - smells like a bug. If that is required then KIO likely…
rominfAuthorUnsubmitted
Done

I understand that this is a bug. I've tried to fix it but I didn't understand how does KDirLister work.
I propose to accept this quick fix for now. Meanwhile, fill a bug report in KIO. Moreover, it would be faster.

rominf: I understand that this is a bug. I've tried to fix it but I didn't understand how does…
markgUnsubmitted
Done

I rather not do that. Quick fixes (nearly) always stay in for far too long or people just forget about it altogether because it "seemingly works".
Lets do it this way, I will have a look at KDirLister to see if i can get this fixed. I'd have to write a valid testcase first that fails which makes finding the bug itself easier. Writing that test is the difficult part i think.

If i haven't found a fix for it by the end of sunday then i'm not going to find one anytime soon.

So: wait till sunday. If i found a fix, use it.
If i found no fix then by all means, use this workaround but put comments around it that it is a workaround and should be fixed in KIO instead.

markg: I rather not do that. Quick fixes (nearly) always stay in for far too long or people just…
rominfAuthorUnsubmitted
Not Done

OK. I agree.

rominf: OK. I agree.
rominfAuthorUnsubmitted
Not Done

Did you find/fix a bug?

rominf: Did you find/fix a bug?
markgUnsubmitted
Done

Don't overuse auto. It makes sense for the lambda above and for otherwise link complicated types. In this case, just use bool.

markg: Don't overuse auto. It makes sense for the lambda above and for otherwise link complicated…