Changeset View
Changeset View
Standalone View
Standalone View
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) { | ||||
kossebau: Isn't m_views destroyed (and thus implicitely cleared) before reaching the ~QWidget and… | |||||
202 | m_stackwidget->disconnect(this); | ||||
203 | } | ||||
204 | } | ||||
205 | | ||||
195 | void OutputWidget::clearModel() | 206 | void OutputWidget::clearModel() | ||
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 | | |||
253 | void 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 | QTreeView *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 | | |||
293 | void OutputWidget::closeActiveView() | 301 | void OutputWidget::closeActiveView() | ||
294 | { | 302 | { | ||
295 | QWidget* widget = m_tabwidget->currentWidget(); | 303 | QWidget* widget = m_tabwidget->currentWidget(); | ||
Show All 38 Lines | 340 | { | |||
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 | | |||
347 | KDevelop::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 | | |||
472 | QTreeView* 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 | auto 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 | | |||
538 | void 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 82 Lines • ▼ Show 20 Line(s) | 635 | { | |||
633 | { | 641 | { | ||
634 | index = m_stackwidget->currentIndex(); | 642 | index = m_stackwidget->currentIndex(); | ||
635 | } | 643 | } | ||
636 | return index; | 644 | return index; | ||
637 | } | 645 | } | ||
638 | 646 | | |||
639 | void OutputWidget::outputFilter(const QString& filter) | 647 | void OutputWidget::outputFilter(const QString& filter) | ||
640 | { | 648 | { | ||
641 | QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget()); | 649 | auto view = qobject_cast<QAbstractItemView*>(currentWidget()); | ||
642 | if( !view ) | 650 | if(!view) { | ||
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. | |||||
643 | return; | 651 | return; | ||
652 | } | ||||
644 | 653 | | |||
645 | auto fvIt = findFilteredView(view); | 654 | auto fvIt = findFilteredView(view); | ||
646 | auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); | 655 | auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); | ||
647 | if( !proxyModel ) | 656 | if(!proxyModel) | ||
648 | { | 657 | { | ||
649 | proxyModel = new QSortFilterProxyModel(view->model()); | 658 | // create new proxy model and make view it's parent. This allows us destroy view and | ||
659 | // it's model with "one shot" (see removeOutput() method). | ||||
660 | fvIt->proxyModel = proxyModel = new QSortFilterProxyModel(view); | ||||
650 | proxyModel->setDynamicSortFilter(true); | 661 | proxyModel->setDynamicSortFilter(true); | ||
651 | proxyModel->setSourceModel(view->model()); | 662 | proxyModel->setSourceModel(view->model()); | ||
652 | fvIt->proxyModel = QSharedPointer<QSortFilterProxyModel>(proxyModel); | | |||
653 | view->setModel(proxyModel); | 663 | view->setModel(proxyModel); | ||
654 | } | 664 | } | ||
655 | QRegExp regExp(filter, Qt::CaseInsensitive); | 665 | proxyModel->setFilterRegExp(QRegExp(filter, Qt::CaseInsensitive)); | ||
kossebau: Unrelated change? | |||||
656 | proxyModel->setFilterRegExp(regExp); | | |||
657 | fvIt->filter = filter; | 666 | fvIt->filter = filter; | ||
658 | } | 667 | } | ||
659 | 668 | | |||
660 | void 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 | | |||
675 | void 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 | | |||
686 | QHash<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 | } |
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?