Differential D18380 Diff 49863 src/filewidgets/kdiroperatordetailview.cpp

Changeset View

Standalone View

View Options

src/filewidgets/kdiroperatordetailview.cpp

Show All 13 Lines
14 * along with this library; see the file COPYING.LIB. If not, write to * 14​ * along with this library; see the file COPYING.LIB. If not, write to *
15 * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, * 15​ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, *
16 * Boston, MA 02110-1301, USA. * 16​ * Boston, MA 02110-1301, USA. *
17 *****************************************************************************/ 17​ *****************************************************************************/
18 18​
19#include "kdiroperatordetailview_p.h" 19​#include "kdiroperatordetailview_p.h"
20 20​
21#include <kdirlister.h> 21​#include <kdirlister.h>
22#include <kdirmodel.h> ​
23#include <QDebug> 22​#include <QDebug>
24 23​
25#include <QEvent> 24​#include <QEvent>
26#include <QTimer> 25​#include <QTimer>
27#include <QAbstractProxyModel> 26​#include <QAbstractProxyModel>
28#include <QApplication> 27​#include <QApplication>
29#include <QHeaderView> 28​#include <QHeaderView>
30#include <QListView> 29​#include <QListView>
Show All 12 Lines37{
43 setSelectionBehavior(QAbstractItemView::SelectRows); 42​ setSelectionBehavior(QAbstractItemView::SelectRows);
44 setEditTriggers(QAbstractItemView::NoEditTriggers); 43​ setEditTriggers(QAbstractItemView::NoEditTriggers);
45 setVerticalScrollMode(QListView::ScrollPerPixel); 44​ setVerticalScrollMode(QListView::ScrollPerPixel);
46 setHorizontalScrollMode(QListView::ScrollPerPixel); 45​ setHorizontalScrollMode(QListView::ScrollPerPixel);
47} 46​}
48 47​
49KDirOperatorDetailView::~KDirOperatorDetailView() 48​KDirOperatorDetailView::~KDirOperatorDetailView()
50{ 49​{
51} 50​}
dfaureUnsubmitted
Not Done

This is a horrible hack, relying on the exact number of parents before getting to KFileWidget.
Either cast inside a while loop (i.e. "go up until a KFileWidget"), or find a way to pass the proper pointer to this class.

dfaure: This is a horrible hack, relying on the exact number of parents before getting to KFileWidget.
52 51​
53bool KDirOperatorDetailView::setViewMode(KFile::FileView viewMode) 52​bool KDirOperatorDetailView::setViewMode(KFile::FileView viewMode)
54{ 53​{
cfeckUnsubmitted
Not Done

Can we use qobject_cast here?

cfeck: Can we use qobject_cast here?
rjvbbAuthorUnsubmitted
Not Done

Doh, of course.

It's probably more expensive (which shouldn't matter here) but also means we have to include kfilewidget.h. Is that OK?

rjvbb: Doh, of course. It's probably more expensive (which shouldn't matter here) but also means we…
55 bool tree = false; 54​ bool tree = false;
56 55​
57 if (KFile::isDetailView(viewMode)) { 56​ if (KFile::isDetailView(viewMode)) {
58 m_hideDetailColumns = false; 57​ m_hideDetailColumns = false;
59 setAlternatingRowColors(true); 58​ setAlternatingRowColors(true);
60 } else if (KFile::isTreeView(viewMode)) { 59​ } else if (KFile::isTreeView(viewMode)) {
61 m_hideDetailColumns = true; 60​ m_hideDetailColumns = true;
62 setAlternatingRowColors(false); 61​ setAlternatingRowColors(false);
63 tree = true; 62​ tree = true;
64 } else if (KFile::isDetailTreeView(viewMode)) { 63​ } else if (KFile::isDetailTreeView(viewMode)) {
65 m_hideDetailColumns = false; 64​ m_hideDetailColumns = false;
66 setAlternatingRowColors(true); 65​ setAlternatingRowColors(true);
67 tree = true; 66​ tree = true;
68 } else { 67​ } else {
69 return false; 68​ return false;
70 } 69​ }
71 70​
72 setRootIsDecorated(tree); 71​ setRootIsDecorated(tree);
73 setItemsExpandable(tree); 72​ setItemsExpandable(tree);
74 // This allows to have a horizontal scrollbar in case this view is used as 73​ // This allows to have a horizontal scrollbar in case this view is used as
75 // a plain treeview instead of cutting off filenames, especially useful when 74​ // a plain treeview instead of cutting off filenames, especially useful when
76 // using KDirOperator in horizontally limited parts of an app. 75​ // using KDirOperator in horizontally limited parts of an app.
77 if (tree && m_hideDetailColumns) { 76​ if (tree && m_hideDetailColumns) {
78 header()->setSectionResizeMode(QHeaderView::ResizeToContents); 77​ header()->setSectionResizeMode(QHeaderView::ResizeToContents);
dfaureUnsubmitted
Not Done

use override, not virtual

dfaure: use override, not virtual
79 } else { 78​ } else {
80 header()->setSectionResizeMode(QHeaderView::Interactive); 79​ header()->setSectionResizeMode(QHeaderView::Interactive);
81 } 80​ }
82 81​
83 return true; 82​ return true;
84} 83​}
85 84​
86bool KDirOperatorDetailView::event(QEvent *event) 85​bool KDirOperatorDetailView::event(QEvent *event)
87{ 86​{
88 if (event->type() == QEvent::Polish) { 87​ switch (event->type()) {
88​ case QEvent::Polish: {
89 QHeaderView *headerView = header(); 89​ QHeaderView *headerView = header();
90 headerView->setSectionResizeMode(0, QHeaderView::Stretch); 90​ headerView->setSectionResizeMode(0, QHeaderView::Stretch);
91 headerView->setSectionResizeMode(1, QHeaderView::ResizeToContents); 91​ headerView->setSectionResizeMode(1, QHeaderView::ResizeToContents);
92 headerView->setSectionResizeMode(2, QHeaderView::ResizeToContents); 92​ headerView->setSectionResizeMode(2, QHeaderView::ResizeToContents);
93 headerView->setStretchLastSection(false); 93​ headerView->setStretchLastSection(false);
94 headerView->setSectionsMovable(false); 94​ headerView->setSectionsMovable(false);
95 95​
96 setColumnHidden(KDirModel::Size, m_hideDetailColumns); 96​ setColumnHidden(KDirModel::Size, m_hideDetailColumns);
97 setColumnHidden(KDirModel::ModifiedTime, m_hideDetailColumns); 97​ setColumnHidden(KDirModel::ModifiedTime, m_hideDetailColumns);
98 hideColumn(KDirModel::Type); 98​ hideColumn(KDirModel::Type);
99 hideColumn(KDirModel::Permissions); 99​ hideColumn(KDirModel::Permissions);
100 hideColumn(KDirModel::Owner); 100​ hideColumn(KDirModel::Owner);
101 hideColumn(KDirModel::Group); 101​ hideColumn(KDirModel::Group);
102 } else if (event->type() == QEvent::UpdateRequest) { 102​ m_setInteractiveResizeMode = false;
103​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
104​ m_currentColumnWidth[i] = -1;
105​ }
106​ connect(header(), &QHeaderView::sectionResized,
107​ [=](int column, int, int newSize) {
108​ if (newSize > 0 && model()->rowCount() > 0) {
109​ // store the new width; note that we may be called multiple times
110​ // for columns in Stretch mode, with increasingly accurate sizes.
111​ m_currentColumnWidth[column] = newSize;
112​ // for some reason making a selective ResizeMode setting here
113​ // has no effect; we need to set interactive mode on all sections
114​ // at a later stage. Queue that now, and don't touch any
115​ // section settings directly.
116​ m_setInteractiveResizeMode = true;
117​ }
118​ });
119​ break;
120​ }
121​ case QEvent::UpdateRequest:
103 // A wheel movement will scroll 4 items 122​ // A wheel movement will scroll 4 items
104 if (model()->rowCount()) { 123​ if (model()->rowCount()) {
105 verticalScrollBar()->setSingleStep((sizeHintForRow(0) / 3) * 4); 124​ verticalScrollBar()->setSingleStep((sizeHintForRow(0) / 3) * 4);
106 } 125​ }
126​ break;
127​ case QEvent::Paint:
128​ // event analysis confirms that the 1st paint event arrives after all headers have
129​ // had a size set, so we can now set the definitive section size explicitly and
130​ // activate interactive mode. The results is that section size is set once according
131​ // to the principles defined above (Polish case) but can then be adapted by the user,
132​ // for instance when navigating to a different directory.
133​ if (m_setInteractiveResizeMode) {
134​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
135​ if (m_currentColumnWidth[i] > 0) {
136​ // set the obtained width explicitly
137​ header()->resizeSection(i, m_currentColumnWidth[i]);
138​ }
139​ }
140​ header()->setSectionResizeMode(QHeaderView::Interactive);
141​ // we only do this once after each Polish event which reset the column size mode.
142​ // (In practice this events seems to arrive only once.)
143​ m_setInteractiveResizeMode = false;
144​ }
145​ break;
146​ default:
147​ // noop
148​ break;
107 } 149​ }
108 150​
109 return QTreeView::event(event); 151​ return QTreeView::event(event);
110} 152​}
111 153​
112void KDirOperatorDetailView::dragEnterEvent(QDragEnterEvent *event) 154​void KDirOperatorDetailView::dragEnterEvent(QDragEnterEvent *event)
113{ 155​{
114 if (event->mimeData()->hasUrls()) { 156​ if (event->mimeData()->hasUrls()) {
115 event->acceptProposedAction(); 157​ event->acceptProposedAction();
116 } 158​ }
117} 159​}
118 160​
119void KDirOperatorDetailView::mousePressEvent(QMouseEvent *event) 161​void KDirOperatorDetailView::mousePressEvent(QMouseEvent *event)
dfaureUnsubmitted
Not Done

Why is this (and the following 12 lines or so) done on every resize? Surely that's not needed?

Split this into case Resize and case Polish, I don't see even one line of code that needs to be in both.

dfaure: Why is this (and the following 12 lines or so) done on every resize? Surely that's not needed?
120{ 162​{
121 QTreeView::mousePressEvent(event); 163​ QTreeView::mousePressEvent(event);
122 164​
123 const QModelIndex index = indexAt(event->pos()); 165​ const QModelIndex index = indexAt(event->pos());
124 if (!index.isValid() || (index.column() != KDirModel::Name)) { 166​ if (!index.isValid() || (index.column() != KDirModel::Name)) {
125 const Qt::KeyboardModifiers modifiers = QApplication::keyboardModifiers(); 167​ const Qt::KeyboardModifiers modifiers = QApplication::keyboardModifiers();
126 if (!(modifiers & Qt::ShiftModifier) && !(modifiers & Qt::ControlModifier)) { 168​ if (!(modifiers & Qt::ShiftModifier) && !(modifiers & Qt::ControlModifier)) {
127 clearSelection(); 169​ clearSelection();
128 } 170​ }
129 } 171​ }
130} 172​}
131 173​
132void KDirOperatorDetailView::currentChanged(const QModelIndex &current, const QModelIndex &previous) 174​void KDirOperatorDetailView::currentChanged(const QModelIndex &current, const QModelIndex &previous)
dfaureUnsubmitted
Not Done

URGGH. This smells like a horrible hack. Widgets do layouting, and then painting. Modifying layouting parameters (hiding columns, resizing columns...) at painting time is very wrong and a recipe for infinite loops (paint -> layout -> paint -> layout...).

The comment talks about a "first paint event" - if that's all you need, then at least add a static bool so that this code is run only once. But I still won't like it very much. Isn't that what Polish is for?

dfaure: URGGH. This smells like a horrible hack. Widgets do layouting, and then painting. Modifying…
133{ 175​{
134 QTreeView::currentChanged(current, previous); 176​ QTreeView::currentChanged(current, previous);
135} 177​}
ngrahamUnsubmitted
Done

This is almost always wrong. Can you explain why you think it's required here?

ngraham: This is almost always wrong. Can you explain why you think it's required here?
rjvbbAuthorUnsubmitted
Not Done

I didn't think anything, I did what an error message told me.

When I remove the include it comes back:

[ 78%] Automatic MOC for target KF5KIOFileWidgets
cd /path/to/build/src/filewidgets && /opt/local/bin/cmake -E cmake_autogen /path/to/build/src/filewidgets/CMakeFiles/KF5KIOFileWidgets_autogen.dir/AutogenInfo.cmake MacPorts
AutoMoc error
-------------
  "/path/to/kio-git/src/filewidgets/kdiroperatordetailview.cpp"
The file contains a Q_OBJECT macro, but does not include "kdiroperatordetailview.moc"!
Consider to
 - add #include "kdiroperatordetailview.moc"
 - enable SKIP_AUTOMOC for this file

make[2]: *** [src/filewidgets/CMakeFiles/KF5KIOFileWidgets_autogen] Error 1

It must be due to the fact that I put the entire KDirHeaderView class definition into the implementation file, which I did because it's really private and changes to it shouldn't trigger a rebuild of a large number of files that are not concerned by those changes at all (kdiroperatordetailview_p.h is included by quite a few files).

rjvbb: I didn't think anything, I did what an error message told me. When I remove the include it…
dfaureUnsubmitted
Not Done

Yeah, this is correct when defining a class with Q_OBJECT in the cpp file. The moc code needs to see the class definition, and the only way to do that is to include the moc.

dfaure: Yeah, this is correct when defining a class with Q_OBJECT in the cpp file. The moc code needs…
dfaureUnsubmitted
Not Done

If you mean Q_ASSERT, use Q_ASSERT. If on the other hand, it can happen for good reasons, remove the qWarning.

dfaure: If you mean Q_ASSERT, use Q_ASSERT. If on the other hand, it can happen for good reasons…