Differential D18380 Diff 49908 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) { ​
89 QHeaderView *headerView = header(); 87​ QHeaderView *headerView = header();
88​ switch (auto type = event->type()) {
89​ case QEvent::Resize:
90​ case QEvent::Polish:
90 headerView->setSectionResizeMode(0, QHeaderView::Stretch); 91​ headerView->setSectionResizeMode(0, QHeaderView::Stretch);
91 headerView->setSectionResizeMode(1, QHeaderView::ResizeToContents); 92​ headerView->setSectionResizeMode(1, QHeaderView::ResizeToContents);
92 headerView->setSectionResizeMode(2, QHeaderView::ResizeToContents); 93​ headerView->setSectionResizeMode(2, QHeaderView::ResizeToContents);
93 headerView->setStretchLastSection(false); 94​ headerView->setStretchLastSection(false);
94 headerView->setSectionsMovable(false); 95​ headerView->setSectionsMovable(false);
95 96​
96 setColumnHidden(KDirModel::Size, m_hideDetailColumns); 97​ setColumnHidden(KDirModel::Size, m_hideDetailColumns);
97 setColumnHidden(KDirModel::ModifiedTime, m_hideDetailColumns); 98​ setColumnHidden(KDirModel::ModifiedTime, m_hideDetailColumns);
98 hideColumn(KDirModel::Type); 99​ hideColumn(KDirModel::Type);
99 hideColumn(KDirModel::Permissions); 100​ hideColumn(KDirModel::Permissions);
100 hideColumn(KDirModel::Owner); 101​ hideColumn(KDirModel::Owner);
101 hideColumn(KDirModel::Group); 102​ hideColumn(KDirModel::Group);
102 } else if (event->type() == QEvent::UpdateRequest) { 103​ m_setInteractiveResizeMode = false;
104​ if (type == QEvent::Polish) {
105​ // the polish event seems to arrive only once during our lifetime so
106​ // this is a good moment for some JIT initialisation.
107​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
108​ m_currentColumnWidth[i] = -1;
109​ }
110​ // install the section resize handler
111​ connect(headerView, &QHeaderView::sectionResized,
112​ [=](int column, int, int newSize) {
113​ if (newSize > 0 && model()->rowCount() > 0) {
114​ // store the new width; note that we may be called multiple times
115​ // for columns in Stretch mode, with increasingly accurate sizes.
116​ m_currentColumnWidth[column] = newSize;
117​ // for some reason making a selective ResizeMode setting here
118​ // has no effect; we need to set interactive mode on all sections
119​ // at a later stage. Queue that now, and don't touch any
120​ // section settings directly.
121​ m_setInteractiveResizeMode = true;
122​ }
123​ });
124​ }
125​ break;
126​ case QEvent::UpdateRequest:
103 // A wheel movement will scroll 4 items 127​ // A wheel movement will scroll 4 items
104 if (model()->rowCount()) { 128​ if (model()->rowCount()) {
105 verticalScrollBar()->setSingleStep((sizeHintForRow(0) / 3) * 4); 129​ verticalScrollBar()->setSingleStep((sizeHintForRow(0) / 3) * 4);
106 } 130​ }
131​ break;
132​ case QEvent::Paint:
133​ // event analysis confirms that the 1st paint event arrives after all headers have
134​ // had a size set, so we can now set the definitive section size explicitly and
135​ // activate interactive mode. The results is that section size is set once according
136​ // to the principles defined above (Polish case) but can then be adapted by the user,
137​ // for instance when navigating to a different directory.
138​ if (m_setInteractiveResizeMode) {
139​ headerView->setSectionResizeMode(QHeaderView::Interactive);
140​ // define a minimum width for the name column, here a
141​ // a third more than the width of the date column.
142​ const int minNameWidth = headerView->sectionSize(2) * 1.33 + 0.5;
143​ // compensation for very narrow views. Here we use sectionSize() to obtain
144​ // current values.
145​ if (headerView->sectionSize(0) > 0 && headerView->sectionSize(0) < minNameWidth) {
146​ m_currentColumnWidth[0] = minNameWidth;
147​ // make the size column a bit (20%) smaller
148​ m_currentColumnWidth[1] = headerView->sectionSize(1) * 4 / 5;
149​ }
150​ // set the values we want to set explicitly
151​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
152​ if (m_currentColumnWidth[i] > 0) {
153​ // set the obtained width explicitly
154​ headerView->resizeSection(i, m_currentColumnWidth[i]);
155​ }
156​ }
157​ headerView->setSectionsMovable(true);
158​ // we only do this once after receiving a Polish or Resize event
159​ // both of which reset the column size mode.
160​ m_setInteractiveResizeMode = false;
161​ }
162​ break;
163​ default:
164​ // noop
165​ break;
107 } 166​ }
108 167​
109 return QTreeView::event(event); 168​ return QTreeView::event(event);
110} 169​}
111 170​
112void KDirOperatorDetailView::dragEnterEvent(QDragEnterEvent *event) 171​void KDirOperatorDetailView::dragEnterEvent(QDragEnterEvent *event)
113{ 172​{
114 if (event->mimeData()->hasUrls()) { 173​ if (event->mimeData()->hasUrls()) {
115 event->acceptProposedAction(); 174​ event->acceptProposedAction();
116 } 175​ }
117} 176​}
118 177​
119void KDirOperatorDetailView::mousePressEvent(QMouseEvent *event) 178​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{ 179​{
121 QTreeView::mousePressEvent(event); 180​ QTreeView::mousePressEvent(event);
122 181​
123 const QModelIndex index = indexAt(event->pos()); 182​ const QModelIndex index = indexAt(event->pos());
124 if (!index.isValid() || (index.column() != KDirModel::Name)) { 183​ if (!index.isValid() || (index.column() != KDirModel::Name)) {
125 const Qt::KeyboardModifiers modifiers = QApplication::keyboardModifiers(); 184​ const Qt::KeyboardModifiers modifiers = QApplication::keyboardModifiers();
126 if (!(modifiers & Qt::ShiftModifier) && !(modifiers & Qt::ControlModifier)) { 185​ if (!(modifiers & Qt::ShiftModifier) && !(modifiers & Qt::ControlModifier)) {
127 clearSelection(); 186​ clearSelection();
128 } 187​ }
129 } 188​ }
130} 189​}
131 190​
132void KDirOperatorDetailView::currentChanged(const QModelIndex &current, const QModelIndex &previous) 191​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{ 192​{
134 QTreeView::currentChanged(current, previous); 193​ QTreeView::currentChanged(current, previous);
135} 194​}
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…