Differential D18380 Diff 50084 src/filewidgets/kdiroperatordetailview.cpp

Changeset View

Standalone View

View Options

src/filewidgets/kdiroperatordetailview.cpp

Show All 26 Lines
27#include <QAbstractProxyModel> 27​#include <QAbstractProxyModel>
28#include <QApplication> 28​#include <QApplication>
29#include <QHeaderView> 29​#include <QHeaderView>
30#include <QListView> 30​#include <QListView>
31#include <QMimeData> 31​#include <QMimeData>
32#include <QResizeEvent> 32​#include <QResizeEvent>
33#include <QScrollBar> 33​#include <QScrollBar>
34 34​
35​class KDirHeaderView : public QHeaderView
36​{
37​ Q_OBJECT
38​public:
39​ explicit KDirHeaderView(KDirOperatorDetailView *view, Qt::Orientation orientation, QWidget *parent = nullptr)
40​ : QHeaderView(orientation, parent),
41​ q(view),
42​ m_minimalNameColumnWidth(-1),
43​ m_setInteractiveResizeMode(false),
44​ m_checkMinimalNameColumnWidth(false),
45​ m_narrowMode(false)
46​ {
47​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
48​ m_currentColumnWidth[i] = -1;
49​ }
50​ auto pw = view->parentWidget();
51​ for (int pn = 0; pw && pn < 3; ++pn) {
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​ pw = pw->parentWidget();
53​ }
54​ m_isFileWidget = pw ? strcmp(pw->metaObject()->className(), "KFileWidget") == 0 : false;
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​ // install the section resize handler
56​ connect(this, &QHeaderView::sectionResized, this, &KDirHeaderView::sectionResizeHandler);
57​ }
58​
59​ void resetWidthTracking(bool all)
60​ {
61​ m_setInteractiveResizeMode = false;
62​ m_checkMinimalNameColumnWidth = true;
63​ if (all) {
64​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
65​ m_currentColumnWidth[i] = -1;
66​ }
67​ m_minimalNameColumnWidth = -1;
68​ }
69​ }
70​
71​ void setMinimumNameColumnWidth()
72​ {
73​ if (m_minimalNameColumnWidth < 0) {
74​ m_minimalNameColumnWidth = sectionSize(2) * 1.33 + 0.5;
75​ }
76​ }
77​
78​ virtual QSize sectionSizeFromContents(int logicalIndex) const
dfaureUnsubmitted
Not Done

use override, not virtual

dfaure: use override, not virtual
79​ {
80​ auto size = QHeaderView::sectionSizeFromContents(logicalIndex);
81​ if (logicalIndex == 0 && size.width() < m_minimalNameColumnWidth) {
82​ size.setWidth(m_minimalNameColumnWidth);
83​ }
84​ return size;
85​ }
86​
87​ void sectionResizeHandler(int column, int, int newSize)
88​ {
89​ if (newSize > 0 && model()->rowCount() > 0) {
90​ if (column == 0 && newSize < m_minimalNameColumnWidth) {
91​ resizeSection(0, m_minimalNameColumnWidth);
92​ } else {
93​ // store the new width; note that we may be called multiple times
94​ // for columns in Stretch mode, with increasingly accurate sizes.
95​ m_currentColumnWidth[column] = newSize;
96​ }
97​ // for some reason making a selective ResizeMode setting here
98​ // has no effect; we need to set interactive mode on all sections
99​ // at a later stage. Queue that now, and don't touch any
100​ // section settings directly.
101​ m_setInteractiveResizeMode = true;
102​ // also set this flag here because we may be called even if the
103​ // user isn't resizing the parent widget.
104​ m_checkMinimalNameColumnWidth = true;
105​ }
106​ }
107​
108​ KDirOperatorDetailView* q;
109​ int m_currentColumnWidth[KDirModel::ColumnCount];
110​ int m_minimalNameColumnWidth;
111​ bool m_setInteractiveResizeMode;
112​ bool m_checkMinimalNameColumnWidth;
113​ bool m_isFileWidget;
114​ bool m_narrowMode;
115​};
116​
35KDirOperatorDetailView::KDirOperatorDetailView(QWidget *parent) : 117​KDirOperatorDetailView::KDirOperatorDetailView(QWidget *parent) :
36 QTreeView(parent), 118​ QTreeView(parent),
37 m_hideDetailColumns(false) 119​ m_hideDetailColumns(false)
38{ 120​{
39 setRootIsDecorated(false); 121​ setRootIsDecorated(false);
40 setSortingEnabled(true); 122​ setSortingEnabled(true);
41 setUniformRowHeights(true); 123​ setUniformRowHeights(true);
42 setDragDropMode(QListView::DragOnly); 124​ setDragDropMode(QListView::DragOnly);
43 setSelectionBehavior(QAbstractItemView::SelectRows); 125​ setSelectionBehavior(QAbstractItemView::SelectRows);
44 setEditTriggers(QAbstractItemView::NoEditTriggers); 126​ setEditTriggers(QAbstractItemView::NoEditTriggers);
45 setVerticalScrollMode(QListView::ScrollPerPixel); 127​ setVerticalScrollMode(QListView::ScrollPerPixel);
46 setHorizontalScrollMode(QListView::ScrollPerPixel); 128​ setHorizontalScrollMode(QListView::ScrollPerPixel);
129​ const auto currentHeader = header();
130​ setHeader(new KDirHeaderView(this, currentHeader->orientation(), currentHeader->parentWidget()));
47} 131​}
48 132​
49KDirOperatorDetailView::~KDirOperatorDetailView() 133​KDirOperatorDetailView::~KDirOperatorDetailView()
50{ 134​{
51} 135​}
52 136​
53bool KDirOperatorDetailView::setViewMode(KFile::FileView viewMode) 137​bool KDirOperatorDetailView::setViewMode(KFile::FileView viewMode)
54{ 138​{
Show All 25 Lines163 } else {
80 header()->setSectionResizeMode(QHeaderView::Interactive); 164​ header()->setSectionResizeMode(QHeaderView::Interactive);
81 } 165​ }
82 166​
83 return true; 167​ return true;
84} 168​}
85 169​
86bool KDirOperatorDetailView::event(QEvent *event) 170​bool KDirOperatorDetailView::event(QEvent *event)
87{ 171​{
88 if (event->type() == QEvent::Polish) { 172​ KDirHeaderView *headerView = qobject_cast<KDirHeaderView*>(header());
89 QHeaderView *headerView = header(); 173​ switch (auto type = event->type()) {
174​ case QEvent::Resize:
175​ case QEvent::Polish:
176​ if (type == QEvent::Polish) {
177​ // the polish event seems to arrive only once during our lifetime so
178​ // this is a good moment for some JIT initialisation.
179​ headerView->resetWidthTracking(true);
180​
181​ } else {
182​ headerView->resetWidthTracking(false);
183​ }
90 headerView->setSectionResizeMode(0, QHeaderView::Stretch); 184​ headerView->setSectionResizeMode(0, QHeaderView::Stretch);
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?
91 headerView->setSectionResizeMode(1, QHeaderView::ResizeToContents); 185​ headerView->setSectionResizeMode(1, QHeaderView::ResizeToContents);
92 headerView->setSectionResizeMode(2, QHeaderView::ResizeToContents); 186​ headerView->setSectionResizeMode(2, QHeaderView::ResizeToContents);
93 headerView->setStretchLastSection(false); 187​ headerView->setStretchLastSection(false);
94 headerView->setSectionsMovable(false); 188​ headerView->setSectionsMovable(false);
95 189​
96 setColumnHidden(KDirModel::Size, m_hideDetailColumns); 190​ setColumnHidden(KDirModel::Size, m_hideDetailColumns);
97 setColumnHidden(KDirModel::ModifiedTime, m_hideDetailColumns); 191​ setColumnHidden(KDirModel::ModifiedTime, m_hideDetailColumns);
98 hideColumn(KDirModel::Type); 192​ hideColumn(KDirModel::Type);
99 hideColumn(KDirModel::Permissions); 193​ hideColumn(KDirModel::Permissions);
100 hideColumn(KDirModel::Owner); 194​ hideColumn(KDirModel::Owner);
101 hideColumn(KDirModel::Group); 195​ hideColumn(KDirModel::Group);
102 } else if (event->type() == QEvent::UpdateRequest) { 196​ if (type == QEvent::Resize && !headerView->m_isFileWidget) {
197​ // force an explicit resize here for column 0; without that,
198​ // this column could be resized to and shown at a width less
199​ // than m_minimalNameColumnWidth. This can happen outside of
200​ // our control when resizing Kate's file browser sidebar in
201​ // detailed tree view. It doesn't happen in KFileWidgets
202​ // (open/save dialogs) which are common enough to detect them
203​ // and skip this potentially expensive operation.
204​ resizeColumnToContents(0);
205​ }
206​ break;
207​ case QEvent::UpdateRequest:
103 // A wheel movement will scroll 4 items 208​ // A wheel movement will scroll 4 items
104 if (model()->rowCount()) { 209​ if (model()->rowCount()) {
105 verticalScrollBar()->setSingleStep((sizeHintForRow(0) / 3) * 4); 210​ verticalScrollBar()->setSingleStep((sizeHintForRow(0) / 3) * 4);
106 } 211​ }
212​ break;
213​ case QEvent::Paint:
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…
214​ // event analysis confirms that the 1st paint event arrives after all headers have
215​ // had a size set, so we can now set the definitive section size explicitly and
216​ // activate interactive mode. The results is that section size is set once according
217​ // to the principles defined above (Polish case) but can then be adapted by the user,
218​ // for instance when navigating to a different directory.
219​
220​ // define a minimum width for the name column when we have the 1st valid reference,
221​ // here a third more than the width of the date column.
222​ if (headerView->m_setInteractiveResizeMode) {
223​ headerView->setMinimumNameColumnWidth();
224​ headerView->setSectionResizeMode(QHeaderView::Interactive);
225​ headerView->setSectionsMovable(true);
226​ // we only do this once after receiving a Polish or Resize event,
227​ // both of which reset the column size mode.
228​ headerView->m_setInteractiveResizeMode = false;
229​ }
230​ if (headerView->m_checkMinimalNameColumnWidth) {
231​ // compensation for very narrow views: impose a minimum name column width and
232​ // reduce letterspacing by a small amount. NB: here we use sectionSize() to obtain
233​ // current widths.
234​ if (headerView->sectionSize(0) > 0) {
235​ if (headerView->sectionSize(0) < headerView->m_minimalNameColumnWidth) {
236​ headerView->m_checkMinimalNameColumnWidth = false;
237​ headerView->m_currentColumnWidth[0] = headerView->m_minimalNameColumnWidth;
238​ headerView->m_narrowMode = true;
239​ } else if (headerView->m_narrowMode) {
240​ // sometimes Qt finds a better (larger) width for the name column even
241​ // when in narrow mode; use it.
242​ if (headerView->sectionSize(0) > headerView->m_minimalNameColumnWidth) {
243​ headerView->m_minimalNameColumnWidth = headerView->sectionSize(0);
244​ qDebug() << "KDirOperatorDetailView: narrow mode off - minimalNameColumnWidth now"
245​ << headerView->m_minimalNameColumnWidth;
246​ }
247​ headerView->m_narrowMode = false;
248​ }
249​ }
250​ }
251​ // set the values we want to set explicitly
252​ for (int i = 0 ; i < KDirModel::ColumnCount ; ++i) {
253​ if (headerView->m_currentColumnWidth[i] > 0) {
254​ headerView->setSectionResizeMode(i, QHeaderView::Interactive);
255​ headerView->resizeSection(i, headerView->m_currentColumnWidth[i]);
256​ }
257​ }
258​ if (headerView->m_currentColumnWidth[0] > 0 && headerView->sectionResizeMode(0) != QHeaderView::Interactive) {
259​ qWarning() << "!!!";
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…
260​ }
261​ // if ever we get still non-resizable sections we could make them resizable
262​ // here if columns 1-3 have been resized explicitly.
263​ break;
264​ default:
265​ // noop
266​ break;
107 } 267​ }
108 ​
109 return QTreeView::event(event); 268​ return QTreeView::event(event);
110} 269​}
111 270​
112void KDirOperatorDetailView::dragEnterEvent(QDragEnterEvent *event) 271​void KDirOperatorDetailView::dragEnterEvent(QDragEnterEvent *event)
113{ 272​{
114 if (event->mimeData()->hasUrls()) { 273​ if (event->mimeData()->hasUrls()) {
115 event->acceptProposedAction(); 274​ event->acceptProposedAction();
116 } 275​ }
Show All 11 Lines283 if (!index.isValid() || (index.column() != KDirModel::Name)) {
128 } 287​ }
129 } 288​ }
130} 289​}
131 290​
132void KDirOperatorDetailView::currentChanged(const QModelIndex &current, const QModelIndex &previous) 291​void KDirOperatorDetailView::currentChanged(const QModelIndex &current, const QModelIndex &previous)
133{ 292​{
134 QTreeView::currentChanged(current, previous); 293​ QTreeView::currentChanged(current, previous);
135} 294​}
295​
296​#include "kdiroperatordetailview.moc"
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…