Differential D18380 Diff 50205 src/filewidgets/kdiroperatordetailview.cpp

Changeset View

Standalone View

View Options

src/filewidgets/kdiroperatordetailview.cpp

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

use override, not virtual

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