Changeset View
Changeset View
Standalone View
Standalone View
plugins/standardoutputview/outputwidget.cpp
| Show First 20 Lines • Show All 228 Lines • ▼ Show 20 Line(s) | 224 | { | |||
|---|---|---|---|---|---|
| 229 | { | 229 | { | ||
| 230 | m_stackwidget->setCurrentWidget( view ); | 230 | m_stackwidget->setCurrentWidget( view ); | ||
| 231 | } | 231 | } | ||
| 232 | } | 232 | } | ||
| 233 | 233 | | |||
| 234 | void OutputWidget::changeDelegate( int id ) | 234 | void OutputWidget::changeDelegate( int id ) | ||
| 235 | { | 235 | { | ||
| 236 | if( data->outputdata.contains( id ) && m_views.contains( id ) ) { | 236 | if( data->outputdata.contains( id ) && m_views.contains( id ) ) { | ||
| 237 | m_views.value(id)->setItemDelegate(data->outputdata.value(id)->delegate); | 237 | m_views.value(id).view->setItemDelegate(data->outputdata.value(id)->delegate); | ||
| 238 | } else { | 238 | } else { | ||
| 239 | addOutput(id); | 239 | addOutput(id); | ||
| 240 | } | 240 | } | ||
| 241 | } | 241 | } | ||
| 242 | 242 | | |||
| 243 | void OutputWidget::changeModel( int id ) | 243 | void OutputWidget::changeModel( int id ) | ||
| 244 | { | 244 | { | ||
| 245 | if( data->outputdata.contains( id ) && m_views.contains( id ) ) | 245 | if( data->outputdata.contains( id ) && m_views.contains( id ) ) | ||
| 246 | { | 246 | { | ||
| 247 | OutputData* od = data->outputdata.value(id); | 247 | OutputData* od = data->outputdata.value(id); | ||
| 248 | m_views.value( id )->setModel(od->model); | 248 | m_views.value(id).view->setModel(od->model); | ||
| 249 | } | 249 | } | ||
| 250 | else | 250 | else | ||
| 251 | { | 251 | { | ||
| 252 | addOutput( id ); | 252 | addOutput( id ); | ||
| 253 | } | 253 | } | ||
| 254 | } | 254 | } | ||
| 255 | 255 | | |||
| 256 | void OutputWidget::removeOutput( int id ) | 256 | void OutputWidget::removeOutput( int id ) | ||
| 257 | { | 257 | { | ||
| 258 | if( data->outputdata.contains( id ) && m_views.contains( id ) ) | 258 | if( data->outputdata.contains( id ) && m_views.contains( id ) ) | ||
| 259 | { | 259 | { | ||
| 260 | QTreeView* view = m_views.value(id); | 260 | QTreeView *view = m_views.value(id).view.data(); | ||
kossebau: KDevelop coding style is to have the * with the type, so please keep that (`QTreeView*`) | |||||
Got it. I understood it vice versa after reading the wiki. Will do it right way from now on. vkorneev: Got it. I understood it vice versa after reading the wiki. Will do it right way from now on. | |||||
| 261 | if( data->type & KDevelop::IOutputView::MultipleView || data->type & KDevelop::IOutputView::HistoryView ) | 261 | if( data->type & KDevelop::IOutputView::MultipleView || data->type & KDevelop::IOutputView::HistoryView ) | ||
| 262 | { | 262 | { | ||
| 263 | if( data->type & KDevelop::IOutputView::MultipleView ) | 263 | if( data->type & KDevelop::IOutputView::MultipleView ) | ||
| 264 | { | 264 | { | ||
| 265 | int idx = m_tabwidget->indexOf( view ); | 265 | int idx = m_tabwidget->indexOf( view ); | ||
| 266 | if( idx != -1 ) | 266 | if (idx != -1) | ||
| 267 | { | 267 | { | ||
| 268 | m_tabwidget->removeTab( idx ); | 268 | m_tabwidget->removeTab( idx ); | ||
| 269 | if( m_proxyModels.contains( id ) ) | | |||
| 270 | { | | |||
| 271 | delete m_proxyModels.take( id ); | | |||
| 272 | m_filters.remove( id ); | | |||
| 273 | } | | |||
| 274 | } | 269 | } | ||
| 275 | } else | 270 | } else | ||
| 276 | { | 271 | { | ||
| 277 | int idx = m_stackwidget->indexOf( view ); | 272 | int idx = m_stackwidget->indexOf( view ); | ||
| 278 | if( idx != -1 && m_proxyModels.contains( id ) ) | 273 | if (idx != -1) | ||
| 279 | { | 274 | { | ||
| 280 | delete m_proxyModels.take( id ); | | |||
| 281 | m_filters.remove( id ); | | |||
| 282 | } | | |||
| 283 | m_stackwidget->removeWidget( view ); | 275 | m_stackwidget->removeWidget(view); | ||
| 284 | } | 276 | } | ||
| 285 | delete view; | 277 | } | ||
| 286 | } else | 278 | } else { // KDevelop::IOutputView::OneView | ||
| 287 | { | 279 | /* TODO: this branch of execution has no result because of the "m_views.remove( id );" | ||
| 288 | m_views.value( id )->setModel( nullptr ); | 280 | * after the if-else block. Need to find out which behavior has sense. | ||
| 289 | m_views.value( id )->setItemDelegate( nullptr ); | 281 | */ | ||
| 290 | if( m_proxyModels.contains( 0 ) ) { | 282 | FilteredView& fview = m_views[id]; | ||
| 291 | delete m_proxyModels.take( 0 ); | 283 | fview.view->setModel(nullptr); | ||
| 292 | m_filters.remove( 0 ); | 284 | fview.view->setItemDelegate(nullptr); | ||
| 285 | if (fview.proxyModel) { | ||||
| 286 | fview.proxyModel = QSharedPointer<QSortFilterProxyModel>(); | ||||
aaronpuchert: You can also write `fview.proxyModel.reset()`. | |||||
| 287 | fview.filter = QString(); | ||||
I think this line is redundant. You're doing m_views.remove(id); outside the if-else stmt anyway(?) kfunk: I think this line is redundant. You're doing `m_views.remove(id);` outside the if-else stmt… | |||||
Yes, thanks, it's redundant, it leaked into the code because I was thinking about the issue, described in the TODO comment below. I'll remove it now. vkorneev: Yes, thanks, it's redundant, it leaked into the code because I was thinking about the issue… | |||||
| 293 | } | 288 | } | ||
This can be simplified in one single lookup: auto view = m_views.take(id); view.destroy(); pino: This can be simplified in one single lookup:
lang=c++
auto view = m_views.take(id);
view. | |||||
| 294 | } | 289 | } | ||
| 295 | m_views.remove( id ); | 290 | m_views.remove(id); | ||
| 296 | emit outputRemoved( data->toolViewId, id ); | 291 | emit outputRemoved( data->toolViewId, id ); | ||
pino: Ditto. | |||||
| 297 | } | 292 | } | ||
| 298 | enableActions(); | 293 | enableActions(); | ||
| 299 | } | 294 | } | ||
| 300 | 295 | | |||
| 301 | void OutputWidget::closeActiveView() | 296 | void OutputWidget::closeActiveView() | ||
| 302 | { | 297 | { | ||
| 303 | QWidget* widget = m_tabwidget->currentWidget(); | 298 | QWidget* widget = m_tabwidget->currentWidget(); | ||
| 304 | if( !widget ) | 299 | if( !widget ) | ||
| 305 | return; | 300 | return; | ||
| 306 | foreach( int id, m_views.keys() ) | 301 | foreach( int id, m_views.keys() ) | ||
| 307 | { | 302 | { | ||
| 308 | if( m_views.value(id) == widget ) | 303 | if (m_views.value(id).view == widget) | ||
| 309 | { | 304 | { | ||
| 310 | OutputData* od = data->outputdata.value(id); | 305 | OutputData* od = data->outputdata.value(id); | ||
| 311 | if( od->behaviour & KDevelop::IOutputView::AllowUserClose ) | 306 | if( od->behaviour & KDevelop::IOutputView::AllowUserClose ) | ||
| 312 | { | 307 | { | ||
| 313 | data->plugin->removeOutput( id ); | 308 | data->plugin->removeOutput( id ); | ||
| 314 | } | 309 | } | ||
| 315 | } | 310 | } | ||
| 316 | } | 311 | } | ||
| 317 | enableActions(); | 312 | enableActions(); | ||
| 318 | } | 313 | } | ||
| 319 | 314 | | |||
| 320 | void OutputWidget::closeOtherViews() | 315 | void OutputWidget::closeOtherViews() | ||
| 321 | { | 316 | { | ||
| 322 | QWidget* widget = m_tabwidget->currentWidget(); | 317 | QWidget* widget = m_tabwidget->currentWidget(); | ||
| 323 | if (!widget) | 318 | if (!widget) | ||
| 324 | return; | 319 | return; | ||
| 325 | 320 | | |||
| 326 | foreach (int id, m_views.keys()) { | 321 | foreach (int id, m_views.keys()) { | ||
| 327 | if (m_views.value(id) == widget) { | 322 | if (m_views.value(id).view == widget) { | ||
| 328 | continue; // leave the active view open | 323 | continue; // leave the active view open | ||
| 329 | } | 324 | } | ||
| 330 | 325 | | |||
| 331 | OutputData* od = data->outputdata.value(id); | 326 | OutputData* od = data->outputdata.value(id); | ||
| 332 | if (od->behaviour & KDevelop::IOutputView::AllowUserClose) { | 327 | if (od->behaviour & KDevelop::IOutputView::AllowUserClose) { | ||
| 333 | data->plugin->removeOutput( id ); | 328 | data->plugin->removeOutput( id ); | ||
| 334 | } | 329 | } | ||
| 335 | } | 330 | } | ||
| 336 | enableActions(); | 331 | enableActions(); | ||
| 337 | } | 332 | } | ||
| 338 | 333 | | |||
| 339 | QWidget* OutputWidget::currentWidget() const | 334 | QWidget* OutputWidget::currentWidget() const | ||
| 340 | { | 335 | { | ||
| 341 | QWidget* widget; | 336 | QWidget* widget; | ||
| 342 | if( data->type & KDevelop::IOutputView::MultipleView ) | 337 | if( data->type & KDevelop::IOutputView::MultipleView ) | ||
| 343 | { | 338 | { | ||
| 344 | widget = m_tabwidget->currentWidget(); | 339 | widget = m_tabwidget->currentWidget(); | ||
| 345 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | 340 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | ||
| 346 | { | 341 | { | ||
| 347 | widget = m_stackwidget->currentWidget(); | 342 | widget = m_stackwidget->currentWidget(); | ||
| 348 | } else | 343 | } else | ||
| 349 | { | 344 | { | ||
| 350 | widget = m_views.begin().value(); | 345 | widget = m_views.begin().value().view.data(); | ||
| 351 | } | 346 | } | ||
| 352 | return widget; | 347 | return widget; | ||
| 353 | } | 348 | } | ||
| 354 | 349 | | |||
| 355 | KDevelop::IOutputViewModel *OutputWidget::outputViewModel() const | 350 | KDevelop::IOutputViewModel *OutputWidget::outputViewModel() const | ||
| 356 | { | 351 | { | ||
| 357 | auto view = qobject_cast<QAbstractItemView*>(currentWidget()); | 352 | auto view = qobject_cast<QAbstractItemView*>(currentWidget()); | ||
| 358 | if( !view || !view->isVisible()) | 353 | if( !view || !view->isVisible()) | ||
| Show All 26 Lines | |||||
| 385 | } | 380 | } | ||
| 386 | 381 | | |||
| 387 | void OutputWidget::activateIndex(const QModelIndex &index, QAbstractItemView *view, KDevelop::IOutputViewModel *iface) | 382 | void OutputWidget::activateIndex(const QModelIndex &index, QAbstractItemView *view, KDevelop::IOutputViewModel *iface) | ||
| 388 | { | 383 | { | ||
| 389 | if( ! index.isValid() ) | 384 | if( ! index.isValid() ) | ||
| 390 | return; | 385 | return; | ||
| 391 | QModelIndex sourceIndex = index; | 386 | QModelIndex sourceIndex = index; | ||
| 392 | QModelIndex viewIndex = index; | 387 | QModelIndex viewIndex = index; | ||
| 393 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 388 | auto fvIt = findFilteredView(view); | ||
| 394 | if( QAbstractProxyModel* proxy = m_proxyModels.value(id) ) { | 389 | if (fvIt != m_views.end() && fvIt->proxyModel) { | ||
| 390 | auto proxy = fvIt->proxyModel; | ||||
| 395 | if ( index.model() == proxy ) { | 391 | if ( index.model() == proxy ) { | ||
| 396 | // index is from the proxy, map it to the source | 392 | // index is from the proxy, map it to the source | ||
| 397 | sourceIndex = proxy->mapToSource(index); | 393 | sourceIndex = proxy->mapToSource(index); | ||
| 398 | } else if (proxy == view->model()) { | 394 | } else if (proxy == view->model()) { | ||
| 399 | // index is from the source, map it to the proxy | 395 | // index is from the source, map it to the proxy | ||
| 400 | viewIndex = proxy->mapFromSource(index); | 396 | viewIndex = proxy->mapFromSource(index); | ||
| 401 | } | 397 | } | ||
| 402 | } | 398 | } | ||
| Show All 30 Lines | |||||
| 433 | { | 429 | { | ||
| 434 | auto view = outputView(); | 430 | auto view = outputView(); | ||
| 435 | auto iface = outputViewModel(); | 431 | auto iface = outputViewModel(); | ||
| 436 | if ( ! view || ! iface ) | 432 | if ( ! view || ! iface ) | ||
| 437 | return; | 433 | return; | ||
| 438 | eventuallyDoFocus(); | 434 | eventuallyDoFocus(); | ||
| 439 | 435 | | |||
| 440 | auto index = view->currentIndex(); | 436 | auto index = view->currentIndex(); | ||
| 441 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 437 | auto fvIt = findFilteredView(view); | ||
| 442 | if (QAbstractProxyModel* proxy = m_proxyModels.value(id)) { | 438 | if (fvIt != m_views.end() && fvIt->proxyModel) { | ||
| 439 | auto proxy = fvIt->proxyModel; | ||||
| 443 | if ( index.model() == proxy ) { | 440 | if ( index.model() == proxy ) { | ||
| 444 | // index is from the proxy, map it to the source | 441 | // index is from the proxy, map it to the source | ||
| 445 | index = proxy->mapToSource(index); | 442 | index = proxy->mapToSource(index); | ||
| 446 | } | 443 | } | ||
| 447 | } | 444 | } | ||
| 448 | 445 | | |||
| 449 | QModelIndex newIndex; | 446 | QModelIndex newIndex; | ||
| 450 | switch (selectionMode) { | 447 | switch (selectionMode) { | ||
| ▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Line(s) | 498 | { | |||
| 518 | { | 515 | { | ||
| 519 | if( m_views.isEmpty() ) | 516 | if( m_views.isEmpty() ) | ||
| 520 | { | 517 | { | ||
| 521 | listview = createHelper(); | 518 | listview = createHelper(); | ||
| 522 | 519 | | |||
| 523 | layout()->addWidget( listview ); | 520 | layout()->addWidget( listview ); | ||
| 524 | } else | 521 | } else | ||
| 525 | { | 522 | { | ||
| 526 | listview = m_views.begin().value(); | 523 | listview = m_views.begin().value().view.data(); | ||
| 527 | newView = false; | 524 | newView = false; | ||
| 528 | } | 525 | } | ||
| 529 | } | 526 | } | ||
| 530 | m_views[id] = listview; | 527 | m_views[id].view = QSharedPointer<QTreeView>(listview); | ||
aaronpuchert: And here you can write `m_views[id].view.reset(listview)`. | |||||
It's not correct, 3 line above you get a raw managed pointer, here you make a distinct managed pointer with same source. It'll result in double deletion when goes out of scope. anthonyfieroni: It's not correct, 3 line above you get a raw managed pointer, here you make a distinct managed… | |||||
As @anthonyfieroni said, there is a bug here: listview = m_views.begin().value().view.data(); takes a war copy of the pointer otherwise controlled by a QSharedPointer value. Please fix this by making listview a QSharedPointer itself, so in case an existing pointer is taking, there will be only one set of QSharedPointer around that. kossebau: As @anthonyfieroni said, there is a bug here:
```
listview = m_views.begin().value().view.data… | |||||
| 531 | 528 | | |||
| 532 | changeModel( id ); | 529 | changeModel( id ); | ||
| 533 | changeDelegate( id ); | 530 | changeDelegate( id ); | ||
| 534 | 531 | | |||
| 535 | if (newView) | 532 | if (newView) | ||
| 536 | listview->scrollToBottom(); | 533 | listview->scrollToBottom(); | ||
| 537 | } else | 534 | } else | ||
| 538 | { | 535 | { | ||
| 539 | listview = m_views.value(id); | 536 | listview = m_views.value(id).view.data(); | ||
| 540 | } | 537 | } | ||
| 541 | enableActions(); | 538 | enableActions(); | ||
| 542 | return listview; | 539 | return listview; | ||
| 543 | } | 540 | } | ||
| 544 | 541 | | |||
| 545 | void OutputWidget::raiseOutput(int id) | 542 | void OutputWidget::raiseOutput(int id) | ||
| 546 | { | 543 | { | ||
| 547 | if( m_views.contains(id) ) | 544 | if( m_views.contains(id) ) | ||
| 548 | { | 545 | { | ||
| 546 | auto view = m_views.value(id).view.data(); | ||||
| 549 | if( data->type & KDevelop::IOutputView::MultipleView ) | 547 | if( data->type & KDevelop::IOutputView::MultipleView ) | ||
| 550 | { | 548 | { | ||
| 551 | int idx = m_tabwidget->indexOf( m_views.value(id) ); | 549 | int idx = m_tabwidget->indexOf(view); | ||
| 552 | if( idx >= 0 ) | 550 | if( idx >= 0 ) | ||
| 553 | { | 551 | { | ||
| 554 | m_tabwidget->setCurrentIndex( idx ); | 552 | m_tabwidget->setCurrentIndex( idx ); | ||
| 555 | } | 553 | } | ||
| 556 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | 554 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | ||
| 557 | { | 555 | { | ||
| 558 | int idx = m_stackwidget->indexOf( m_views.value(id) ); | 556 | int idx = m_stackwidget->indexOf(view); | ||
| 559 | if( idx >= 0 ) | 557 | if( idx >= 0 ) | ||
| 560 | { | 558 | { | ||
| 561 | m_stackwidget->setCurrentIndex( idx ); | 559 | m_stackwidget->setCurrentIndex( idx ); | ||
| 562 | } | 560 | } | ||
| 563 | } | 561 | } | ||
| 564 | } | 562 | } | ||
| 565 | enableActions(); | 563 | enableActions(); | ||
| 566 | } | 564 | } | ||
| ▲ Show 20 Lines • Show All 76 Lines • ▼ Show 20 Line(s) | |||||
| 643 | } | 641 | } | ||
| 644 | 642 | | |||
| 645 | void OutputWidget::outputFilter(const QString& filter) | 643 | void OutputWidget::outputFilter(const QString& filter) | ||
| 646 | { | 644 | { | ||
| 647 | QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget()); | 645 | QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget()); | ||
| 648 | if( !view ) | 646 | if( !view ) | ||
| 649 | return; | 647 | return; | ||
| 650 | 648 | | |||
| 651 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 649 | auto fvIt = findFilteredView(view); | ||
| 652 | auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); | 650 | auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); | ||
| 653 | if( !proxyModel ) | 651 | if( !proxyModel ) | ||
| 654 | { | 652 | { | ||
| 655 | proxyModel = new QSortFilterProxyModel(view->model()); | 653 | proxyModel = new QSortFilterProxyModel(view->model()); | ||
| 656 | proxyModel->setDynamicSortFilter(true); | 654 | proxyModel->setDynamicSortFilter(true); | ||
| 657 | proxyModel->setSourceModel(view->model()); | 655 | proxyModel->setSourceModel(view->model()); | ||
| 658 | m_proxyModels.insert(id, proxyModel); | 656 | fvIt->proxyModel = QSharedPointer<QSortFilterProxyModel>(proxyModel); | ||
| 659 | view->setModel(proxyModel); | 657 | view->setModel(proxyModel); | ||
| 660 | } | 658 | } | ||
| 661 | QRegExp regExp(filter, Qt::CaseInsensitive); | 659 | QRegExp regExp(filter, Qt::CaseInsensitive); | ||
| 662 | proxyModel->setFilterRegExp(regExp); | 660 | proxyModel->setFilterRegExp(regExp); | ||
| 663 | m_filters[id] = filter; | 661 | fvIt->filter = filter; | ||
| 664 | } | 662 | } | ||
| 665 | 663 | | |||
| 666 | void OutputWidget::updateFilter(int index) | 664 | void OutputWidget::updateFilter(int index) | ||
| 667 | { | 665 | { | ||
| 668 | QWidget *view = (data->type & KDevelop::IOutputView::MultipleView) | 666 | QWidget *view = (data->type & KDevelop::IOutputView::MultipleView) | ||
| 669 | ? m_tabwidget->widget(index) : m_stackwidget->widget(index); | 667 | ? m_tabwidget->widget(index) : m_stackwidget->widget(index); | ||
| 670 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 668 | auto fvIt = findFilteredView(qobject_cast<QAbstractItemView*>(view)); | ||
| 671 | 669 | | |||
| 672 | if(m_filters.contains(id)) | 670 | if (fvIt != m_views.end() && !fvIt->filter.isEmpty()) | ||
| 673 | { | 671 | { | ||
| 674 | m_filterInput->setText(m_filters[id]); | 672 | m_filterInput->setText(fvIt->filter); | ||
| 675 | } else | 673 | } else | ||
| 676 | { | 674 | { | ||
| 677 | m_filterInput->clear(); | 675 | m_filterInput->clear(); | ||
| 678 | } | 676 | } | ||
| 679 | } | 677 | } | ||
| 680 | 678 | | |||
| 681 | void OutputWidget::setTitle(int outputId, const QString& title) | 679 | void OutputWidget::setTitle(int outputId, const QString& title) | ||
| 682 | { | 680 | { | ||
| 683 | QTreeView* view = m_views.value(outputId, nullptr); | 681 | auto fview = m_views.value(outputId, FilteredView{}); | ||
| 684 | if(view && (data->type & KDevelop::IOutputView::MultipleView)) { | 682 | if (fview.view && (data->type & KDevelop::IOutputView::MultipleView)) { | ||
| 685 | int idx = m_tabwidget->indexOf(view); | 683 | int idx = m_tabwidget->indexOf(fview.view.data()); | ||
| 686 | if (idx >= 0) { | 684 | if (idx >= 0) { | ||
| 687 | m_tabwidget->setTabText(idx, title); | 685 | m_tabwidget->setTabText(idx, title); | ||
| 688 | } | 686 | } | ||
| 689 | } | 687 | } | ||
| 690 | } | 688 | } | ||
| 689 | | ||||
| 690 | QHash<int, OutputWidget::FilteredView>::iterator OutputWidget::findFilteredView(QAbstractItemView *view) | ||||
| 691 | { | ||||
| 692 | for (auto it = m_views.begin(); it != m_views.end(); ++it) { | ||||
| 693 | if (it->view == view) { | ||||
| 694 | return it; | ||||
| 695 | } | ||||
| 696 | } | ||||
| 697 | return m_views.end(); | ||||
| 698 | } | ||||
delete is a no-op on null pointers, so you can simply do: delete foo; foo = nullptr; Of course, assuming you do not need to use the pointer just before its deletion. pino: `delete` is a no-op on null pointers, so you can simply do:
lang=c++
delete foo;
foo =… | |||||
KDevelop coding style is to have the * with the type, so please keep that (QTreeView*)