Differential D15326 Diff 41637 plugins/standardoutputview/outputwidget.cpp

Changeset View

Standalone View

View Options

plugins/standardoutputview/outputwidget.cpp

Show First 20 Lines • Show All 186 Lines • ▼ Show 20 Line(s)68{
187 foreach( int id, data->outputdata.keys() ) 187​ foreach( int id, data->outputdata.keys() )
188 { 188​ {
189 changeModel( id ); 189​ changeModel( id );
190 changeDelegate( id ); 190​ changeDelegate( id );
191 } 191​ }
192 enableActions(); 192​ enableActions();
193} 193​}
194 194​
195​OutputWidget::~OutputWidget()
196​{
197​ // Disconnect our widget to prevent updateFilter() slot calling from parent's destructor,
198​ // which leads to segfault since m_views hash will be destroyed before.
199​ if (m_tabwidget) {
200​ m_tabwidget->disconnect(this);
201​ } else if (m_stackwidget) {
kossebauUnsubmitted
Not Done

Isn't m_views destroyed (and thus implicitely cleared) before reaching the ~QWidget and ~QObject destructors where then any QObject-child-tree memory management is done?
So do we really need to explicitly call this here?

kossebau: Isn't m_views destroyed (and thus implicitely cleared) before reaching the ~QWidget and…
202​ m_stackwidget->disconnect(this);
203​ }
204​}
205​
195void OutputWidget::clearModel() 206​void OutputWidget::clearModel()
kossebauUnsubmitted
Not Done

This here might be more the actual issue, preventing the updateFilter slot being invoked during destruction because tabwidget or stackwidget emitting signals on their subwidgets being removed,

Not sure what is the better approach, only disconnecting from the currentChanged signals or straight deleting the widgets. Personally I tend to only handle the connections, and leave memory management of widgets completly to the Qt internals.

kossebau: This here might be more the actual issue, preventing the updateFilter slot being invoked during…
196{ 207​{
197 auto view = qobject_cast<QAbstractItemView*>(currentWidget()); 208​ auto view = qobject_cast<QAbstractItemView*>(currentWidget());
198 if( !view || !view->isVisible()) 209​ if( !view || !view->isVisible())
199 return; 210​ return;
200 211​
201 KDevelop::OutputModel *outputModel = nullptr; 212​ KDevelop::OutputModel *outputModel = nullptr;
202 if (auto proxy = qobject_cast<QAbstractProxyModel*>(view->model())) { 213​ if (auto proxy = qobject_cast<QAbstractProxyModel*>(view->model())) {
203 outputModel = qobject_cast<KDevelop::OutputModel*>(proxy->sourceModel()); 214​ outputModel = qobject_cast<KDevelop::OutputModel*>(proxy->sourceModel());
▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s)259 {
249 addOutput( id ); 260​ addOutput( id );
250 } 261​ }
251} 262​}
252 263​
253void OutputWidget::removeOutput( int id ) 264​void OutputWidget::removeOutput( int id )
254{ 265​{
255 if( data->outputdata.contains( id ) && m_views.contains( id ) ) 266​ if( data->outputdata.contains( id ) && m_views.contains( id ) )
256 { 267​ {
257 QTreeView *view = m_views.value(id).view.data(); 268​ auto view = m_views.value(id).view;
258 if( data->type & KDevelop::IOutputView::MultipleView || data->type & KDevelop::IOutputView::HistoryView ) 269​ if( data->type & KDevelop::IOutputView::MultipleView || data->type & KDevelop::IOutputView::HistoryView )
259 { 270​ {
260 if( data->type & KDevelop::IOutputView::MultipleView ) 271​ if( data->type & KDevelop::IOutputView::MultipleView )
261 { 272​ {
262 int idx = m_tabwidget->indexOf( view ); 273​ int idx = m_tabwidget->indexOf( view );
263 if (idx != -1) 274​ if (idx != -1)
264 { 275​ {
265 m_tabwidget->removeTab( idx ); 276​ m_tabwidget->removeTab( idx );
266 } 277​ }
267 } else 278​ } else
268 { 279​ {
269 int idx = m_stackwidget->indexOf( view ); 280​ int idx = m_stackwidget->indexOf( view );
270 if (idx != -1) 281​ if (idx != -1)
271 { 282​ {
272 m_stackwidget->removeWidget(view); 283​ m_stackwidget->removeWidget(view);
273 } 284​ }
274 } 285​ }
275 } else { // KDevelop::IOutputView::OneView 286​ } else {
276 /* TODO: this branch of execution has no result because of the "m_views.remove( id );" 287​ // KDevelop::IOutputView::OneView case
277 * after the if-else block. Need to find out which behavior has sense. 288​ // Do nothig here since our single view will be automatically removed from layout
278 */ 289​ // during it's destroy
279 FilteredView& fview = m_views[id]; ​
280 fview.view->setModel(nullptr); ​
281 fview.view->setItemDelegate(nullptr); ​
282 if (fview.proxyModel) { ​
283 fview.proxyModel = QSharedPointer<QSortFilterProxyModel>(); ​
284 fview.filter = QString(); ​
285 } ​
286 } 290​ }
287 m_views.remove(id); 291​
292​ auto fv = m_views.take(id);
293​ // remove our view with proxy model which is view's child (see outputFilter() method).
294​ delete fv.view;
295​
288 emit outputRemoved( data->toolViewId, id ); 296​ emit outputRemoved( data->toolViewId, id );
289 } 297​ }
290 enableActions(); 298​ enableActions();
291} 299​}
292 300​
293void OutputWidget::closeActiveView() 301​void OutputWidget::closeActiveView()
294{ 302​{
295 QWidget* widget = m_tabwidget->currentWidget(); 303​ QWidget* widget = m_tabwidget->currentWidget();
Show All 38 Lines340{
334 if( data->type & KDevelop::IOutputView::MultipleView ) 342​ if( data->type & KDevelop::IOutputView::MultipleView )
335 { 343​ {
336 widget = m_tabwidget->currentWidget(); 344​ widget = m_tabwidget->currentWidget();
337 } else if( data->type & KDevelop::IOutputView::HistoryView ) 345​ } else if( data->type & KDevelop::IOutputView::HistoryView )
338 { 346​ {
339 widget = m_stackwidget->currentWidget(); 347​ widget = m_stackwidget->currentWidget();
340 } else 348​ } else
341 { 349​ {
342 widget = m_views.begin().value().view.data(); 350​ widget = m_views.begin()->view;
343 } 351​ }
344 return widget; 352​ return widget;
345} 353​}
346 354​
347KDevelop::IOutputViewModel *OutputWidget::outputViewModel() const 355​KDevelop::IOutputViewModel *OutputWidget::outputViewModel() const
348{ 356​{
349 auto view = qobject_cast<QAbstractItemView*>(currentWidget()); 357​ auto view = qobject_cast<QAbstractItemView*>(currentWidget());
350 if( !view || !view->isVisible()) 358​ if( !view || !view->isVisible())
▲ Show 20 Lines • Show All 115 Lines • ▼ Show 20 Line(s)472{
466 auto view = outputView(); 474​ auto view = outputView();
467 if( ! view || ! iface ) 475​ if( ! view || ! iface )
468 return; 476​ return;
469 activateIndex(index, view, iface); 477​ activateIndex(index, view, iface);
470} 478​}
471 479​
472QTreeView* OutputWidget::createListView(int id) 480​QTreeView* OutputWidget::createListView(int id)
473{ 481​{
474 auto createHelper = [&]() -> QSharedPointer<QTreeView> { 482​ auto createHelper = [&]() -> QTreeView* {
475 KDevelop::FocusedTreeView* listview = new KDevelop::FocusedTreeView(this); 483​ KDevelop::FocusedTreeView* listview = new KDevelop::FocusedTreeView(this);
476 listview->setEditTriggers( QAbstractItemView::NoEditTriggers ); 484​ listview->setEditTriggers( QAbstractItemView::NoEditTriggers );
477 listview->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn); //Always enable the scrollbar, so it doesn't flash around 485​ listview->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOn); //Always enable the scrollbar, so it doesn't flash around
478 listview->setHeaderHidden(true); 486​ listview->setHeaderHidden(true);
479 listview->setUniformRowHeights(true); 487​ listview->setUniformRowHeights(true);
480 listview->setRootIsDecorated(false); 488​ listview->setRootIsDecorated(false);
481 listview->setSelectionMode( QAbstractItemView::ContiguousSelection ); 489​ listview->setSelectionMode( QAbstractItemView::ContiguousSelection );
482 490​
483 if (data->outputdata.value(id)->behaviour & KDevelop::IOutputView::AutoScroll) { 491​ if (data->outputdata.value(id)->behaviour & KDevelop::IOutputView::AutoScroll) {
484 listview->setAutoScrollAtEnd(true); 492​ listview->setAutoScrollAtEnd(true);
485 } 493​ }
486 494​
487 connect(listview, &QTreeView::activated, this, &OutputWidget::activate); 495​ connect(listview, &QTreeView::activated, this, &OutputWidget::activate);
488 connect(listview, &QTreeView::clicked, this, &OutputWidget::activate); 496​ connect(listview, &QTreeView::clicked, this, &OutputWidget::activate);
489 497​
490 return QSharedPointer<QTreeView>(listview); 498​ return listview;
491 }; 499​ };
492 500​
493 QSharedPointer<QTreeView> listview; 501​ QTreeView* listview = nullptr;
494 if( !m_views.contains(id) ) 502​ if( !m_views.contains(id) )
495 { 503​ {
496 bool newView = true; 504​ bool newView = true;
497 505​
498 if( data->type & KDevelop::IOutputView::MultipleView || data->type & KDevelop::IOutputView::HistoryView ) 506​ if( data->type & KDevelop::IOutputView::MultipleView || data->type & KDevelop::IOutputView::HistoryView )
499 { 507​ {
500 qCDebug(PLUGIN_STANDARDOUTPUTVIEW) << "creating listview"; 508​ qCDebug(PLUGIN_STANDARDOUTPUTVIEW) << "creating listview";
501 listview = createHelper(); 509​ listview = createHelper();
502 510​
503 if( data->type & KDevelop::IOutputView::MultipleView ) 511​ if( data->type & KDevelop::IOutputView::MultipleView )
504 { 512​ {
505 m_tabwidget->addTab(listview.data(), data->outputdata.value(id)->title); 513​ m_tabwidget->addTab(listview, data->outputdata.value(id)->title);
506 } else 514​ } else
507 { 515​ {
508 m_stackwidget->addWidget(listview.data()); 516​ const int index = m_stackwidget->addWidget(listview);
509 m_stackwidget->setCurrentWidget(listview.data()); 517​ m_stackwidget->setCurrentIndex(index);
510 } 518​ }
511 } else 519​ } else
512 { 520​ {
513 if( m_views.isEmpty() ) 521​ if( m_views.isEmpty() )
514 { 522​ {
515 listview = createHelper(); 523​ listview = createHelper();
516 layout()->addWidget(listview.data()); 524​ layout()->addWidget(listview);
517 } else 525​ } else
518 { 526​ {
519 listview = m_views.begin().value().view; 527​ listview = m_views.begin()->view;
520 newView = false; 528​ newView = false;
521 } 529​ }
522 } 530​ }
523 m_views[id].view = listview; 531​ m_views[id].view = listview;
524 532​
525 changeModel( id ); 533​ changeModel( id );
526 changeDelegate( id ); 534​ changeDelegate( id );
527 535​
528 if (newView) 536​ if (newView)
529 listview->scrollToBottom(); 537​ listview->scrollToBottom();
530 } else 538​ } else
531 { 539​ {
532 listview = m_views.value(id).view; 540​ listview = m_views.value(id).view;
533 } 541​ }
534 enableActions(); 542​ enableActions();
535 return listview.data(); 543​ return listview;
536} 544​}
537 545​
538void OutputWidget::raiseOutput(int id) 546​void OutputWidget::raiseOutput(int id)
539{ 547​{
540 if( m_views.contains(id) ) 548​ if( m_views.contains(id) )
541 { 549​ {
542 auto view = m_views.value(id).view.data(); 550​ auto view = m_views.value(id).view;
543 if( data->type & KDevelop::IOutputView::MultipleView ) 551​ if( data->type & KDevelop::IOutputView::MultipleView )
544 { 552​ {
545 int idx = m_tabwidget->indexOf(view); 553​ int idx = m_tabwidget->indexOf(view);
546 if( idx >= 0 ) 554​ if( idx >= 0 )
547 { 555​ {
548 m_tabwidget->setCurrentIndex( idx ); 556​ m_tabwidget->setCurrentIndex( idx );
549 } 557​ }
550 } else if( data->type & KDevelop::IOutputView::HistoryView ) 558​ } else if( data->type & KDevelop::IOutputView::HistoryView )
▲ Show 20 Lines • Show All 84 Lines • ▼ Show 20 Line(s)635{
635 } 643​ }
636 return index; 644​ return index;
637} 645​}
638 646​
639void OutputWidget::outputFilter(const QString& filter) 647​void OutputWidget::outputFilter(const QString& filter)
640{ 648​{
641 QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget()); 649​ QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget());
642 if( !view ) 650​ if( !view )
643 return; 651​ return;
kossebauUnsubmitted
Done

Please no drive-by changes to lines not touched otherwise, especially non-whitespace changes. That make the "git blame" annotations confusing.

kossebau: Please no drive-by changes to lines not touched otherwise, especially non-whitespace changes.
644 652​
645 auto fvIt = findFilteredView(view); 653​ auto fvIt = findFilteredView(view);
646 auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); 654​ auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model());
647 if( !proxyModel ) 655​ if(!proxyModel)
648 { 656​ {
649 proxyModel = new QSortFilterProxyModel(view->model()); 657​ // create new proxy model and make view it's parent. This allows us destroy view and
658​ // it's model with "one shot" (see removeOutput() method).
659​ fvIt->proxyModel = proxyModel = new QSortFilterProxyModel(view);
650 proxyModel->setDynamicSortFilter(true); 660​ proxyModel->setDynamicSortFilter(true);
651 proxyModel->setSourceModel(view->model()); 661​ proxyModel->setSourceModel(view->model());
652 fvIt->proxyModel = QSharedPointer<QSortFilterProxyModel>(proxyModel); ​
653 view->setModel(proxyModel); 662​ view->setModel(proxyModel);
654 } 663​ }
655 QRegExp regExp(filter, Qt::CaseInsensitive); 664​ QRegExp regExp(filter, Qt::CaseInsensitive);
656 proxyModel->setFilterRegExp(regExp); 665​ proxyModel->setFilterRegExp(regExp);
kossebauUnsubmitted
Done

Unrelated change?

kossebau: Unrelated change?
657 fvIt->filter = filter; 666​ fvIt->filter = filter;
658} 667​}
659 668​
660void OutputWidget::updateFilter(int index) 669​void OutputWidget::updateFilter(int index)
661{ 670​{
662 QWidget *view = (data->type & KDevelop::IOutputView::MultipleView) 671​ QWidget *view = (data->type & KDevelop::IOutputView::MultipleView)
663 ? m_tabwidget->widget(index) : m_stackwidget->widget(index); 672​ ? m_tabwidget->widget(index) : m_stackwidget->widget(index);
664 auto fvIt = findFilteredView(qobject_cast<QAbstractItemView*>(view)); 673​ auto fvIt = findFilteredView(qobject_cast<QAbstractItemView*>(view));
665 674​
666 if (fvIt != m_views.end() && !fvIt->filter.isEmpty()) 675​ if (fvIt != m_views.end() && !fvIt->filter.isEmpty())
667 { 676​ {
668 m_filterInput->setText(fvIt->filter); 677​ m_filterInput->setText(fvIt->filter);
669 } else 678​ } else
670 { 679​ {
671 m_filterInput->clear(); 680​ m_filterInput->clear();
672 } 681​ }
673} 682​}
674 683​
675void OutputWidget::setTitle(int outputId, const QString& title) 684​void OutputWidget::setTitle(int outputId, const QString& title)
676{ 685​{
677 auto fview = m_views.value(outputId, FilteredView{}); 686​ auto fview = m_views.value(outputId, FilteredView{});
678 if (fview.view && (data->type & KDevelop::IOutputView::MultipleView)) { 687​ if (fview.view && (data->type & KDevelop::IOutputView::MultipleView)) {
679 int idx = m_tabwidget->indexOf(fview.view.data()); 688​ const int idx = m_tabwidget->indexOf(fview.view);
680 if (idx >= 0) { 689​ if (idx >= 0) {
681 m_tabwidget->setTabText(idx, title); 690​ m_tabwidget->setTabText(idx, title);
682 } 691​ }
683 } 692​ }
684} 693​}
685 694​
686QHash<int, OutputWidget::FilteredView>::iterator OutputWidget::findFilteredView(QAbstractItemView *view) 695​QHash<int, OutputWidget::FilteredView>::iterator OutputWidget::findFilteredView(QAbstractItemView *view)
687{ 696​{
688 for (auto it = m_views.begin(); it != m_views.end(); ++it) { 697​ for (auto it = m_views.begin(); it != m_views.end(); ++it) {
689 if (it->view == view) { 698​ if (it->view == view) {
690 return it; 699​ return it;
691 } 700​ }
692 } 701​ }
693 return m_views.end(); 702​ return m_views.end();
694} 703​}