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