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 | m_views.remove(id); | ||
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… | |||||
| 287 | { | 279 | } else { // KDevelop::IOutputView::OneView | ||
| 288 | m_views.value( id )->setModel( nullptr ); | 280 | /* TODO: this branch of execution has no result because of the "m_views.remove( id );" | ||
| 289 | m_views.value( id )->setItemDelegate( nullptr ); | 281 | * after the if-else block. Need to find out which behavior has sense. | ||
| 290 | if( m_proxyModels.contains( 0 ) ) { | 282 | */ | ||
| 291 | delete m_proxyModels.take( 0 ); | 283 | FilteredView& fview = m_views[id]; | ||
| 292 | m_filters.remove( 0 ); | 284 | fview.view->setModel(nullptr); | ||
| 285 | fview.view->setItemDelegate(nullptr); | ||||
| 286 | if (fview.proxyModel) { | ||||
aaronpuchert: You can also write `fview.proxyModel.reset()`. | |||||
| 287 | fview.proxyModel = QSharedPointer<QSortFilterProxyModel>(); | ||||
| 288 | fview.filter = QString(); | ||||
| 293 | } | 289 | } | ||
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 | } | 290 | } | ||
| 295 | m_views.remove( id ); | 291 | m_views.remove(id); | ||
| 296 | emit outputRemoved( data->toolViewId, id ); | 292 | emit outputRemoved( data->toolViewId, id ); | ||
pino: Ditto. | |||||
| 297 | } | 293 | } | ||
| 298 | enableActions(); | 294 | enableActions(); | ||
| 299 | } | 295 | } | ||
| 300 | 296 | | |||
| 301 | void OutputWidget::closeActiveView() | 297 | void OutputWidget::closeActiveView() | ||
| 302 | { | 298 | { | ||
| 303 | QWidget* widget = m_tabwidget->currentWidget(); | 299 | QWidget* widget = m_tabwidget->currentWidget(); | ||
| 304 | if( !widget ) | 300 | if( !widget ) | ||
| 305 | return; | 301 | return; | ||
| 306 | foreach( int id, m_views.keys() ) | 302 | foreach( int id, m_views.keys() ) | ||
| 307 | { | 303 | { | ||
| 308 | if( m_views.value(id) == widget ) | 304 | if (m_views.value(id).view == widget) | ||
| 309 | { | 305 | { | ||
| 310 | OutputData* od = data->outputdata.value(id); | 306 | OutputData* od = data->outputdata.value(id); | ||
| 311 | if( od->behaviour & KDevelop::IOutputView::AllowUserClose ) | 307 | if( od->behaviour & KDevelop::IOutputView::AllowUserClose ) | ||
| 312 | { | 308 | { | ||
| 313 | data->plugin->removeOutput( id ); | 309 | data->plugin->removeOutput( id ); | ||
| 314 | } | 310 | } | ||
| 315 | } | 311 | } | ||
| 316 | } | 312 | } | ||
| 317 | enableActions(); | 313 | enableActions(); | ||
| 318 | } | 314 | } | ||
| 319 | 315 | | |||
| 320 | void OutputWidget::closeOtherViews() | 316 | void OutputWidget::closeOtherViews() | ||
| 321 | { | 317 | { | ||
| 322 | QWidget* widget = m_tabwidget->currentWidget(); | 318 | QWidget* widget = m_tabwidget->currentWidget(); | ||
| 323 | if (!widget) | 319 | if (!widget) | ||
| 324 | return; | 320 | return; | ||
| 325 | 321 | | |||
| 326 | foreach (int id, m_views.keys()) { | 322 | foreach (int id, m_views.keys()) { | ||
| 327 | if (m_views.value(id) == widget) { | 323 | if (m_views.value(id).view == widget) { | ||
| 328 | continue; // leave the active view open | 324 | continue; // leave the active view open | ||
| 329 | } | 325 | } | ||
| 330 | 326 | | |||
| 331 | OutputData* od = data->outputdata.value(id); | 327 | OutputData* od = data->outputdata.value(id); | ||
| 332 | if (od->behaviour & KDevelop::IOutputView::AllowUserClose) { | 328 | if (od->behaviour & KDevelop::IOutputView::AllowUserClose) { | ||
| 333 | data->plugin->removeOutput( id ); | 329 | data->plugin->removeOutput( id ); | ||
| 334 | } | 330 | } | ||
| 335 | } | 331 | } | ||
| 336 | enableActions(); | 332 | enableActions(); | ||
| 337 | } | 333 | } | ||
| 338 | 334 | | |||
| 339 | QWidget* OutputWidget::currentWidget() const | 335 | QWidget* OutputWidget::currentWidget() const | ||
| 340 | { | 336 | { | ||
| 341 | QWidget* widget; | 337 | QWidget* widget; | ||
| 342 | if( data->type & KDevelop::IOutputView::MultipleView ) | 338 | if( data->type & KDevelop::IOutputView::MultipleView ) | ||
| 343 | { | 339 | { | ||
| 344 | widget = m_tabwidget->currentWidget(); | 340 | widget = m_tabwidget->currentWidget(); | ||
| 345 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | 341 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | ||
| 346 | { | 342 | { | ||
| 347 | widget = m_stackwidget->currentWidget(); | 343 | widget = m_stackwidget->currentWidget(); | ||
| 348 | } else | 344 | } else | ||
| 349 | { | 345 | { | ||
| 350 | widget = m_views.begin().value(); | 346 | widget = m_views.begin().value().view.data(); | ||
| 351 | } | 347 | } | ||
| 352 | return widget; | 348 | return widget; | ||
| 353 | } | 349 | } | ||
| 354 | 350 | | |||
| 355 | KDevelop::IOutputViewModel *OutputWidget::outputViewModel() const | 351 | KDevelop::IOutputViewModel *OutputWidget::outputViewModel() const | ||
| 356 | { | 352 | { | ||
| 357 | auto view = qobject_cast<QAbstractItemView*>(currentWidget()); | 353 | auto view = qobject_cast<QAbstractItemView*>(currentWidget()); | ||
| 358 | if( !view || !view->isVisible()) | 354 | if( !view || !view->isVisible()) | ||
| Show All 26 Lines | |||||
| 385 | } | 381 | } | ||
| 386 | 382 | | |||
| 387 | void OutputWidget::activateIndex(const QModelIndex &index, QAbstractItemView *view, KDevelop::IOutputViewModel *iface) | 383 | void OutputWidget::activateIndex(const QModelIndex &index, QAbstractItemView *view, KDevelop::IOutputViewModel *iface) | ||
| 388 | { | 384 | { | ||
| 389 | if( ! index.isValid() ) | 385 | if( ! index.isValid() ) | ||
| 390 | return; | 386 | return; | ||
| 391 | QModelIndex sourceIndex = index; | 387 | QModelIndex sourceIndex = index; | ||
| 392 | QModelIndex viewIndex = index; | 388 | QModelIndex viewIndex = index; | ||
| 393 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 389 | auto fvIt = findFilteredView(view); | ||
| 394 | if( QAbstractProxyModel* proxy = m_proxyModels.value(id) ) { | 390 | if (fvIt != m_views.end() && fvIt->proxyModel) { | ||
| 391 | auto proxy = fvIt->proxyModel; | ||||
| 395 | if ( index.model() == proxy ) { | 392 | if ( index.model() == proxy ) { | ||
| 396 | // index is from the proxy, map it to the source | 393 | // index is from the proxy, map it to the source | ||
| 397 | sourceIndex = proxy->mapToSource(index); | 394 | sourceIndex = proxy->mapToSource(index); | ||
| 398 | } else if (proxy == view->model()) { | 395 | } else if (proxy == view->model()) { | ||
| 399 | // index is from the source, map it to the proxy | 396 | // index is from the source, map it to the proxy | ||
| 400 | viewIndex = proxy->mapFromSource(index); | 397 | viewIndex = proxy->mapFromSource(index); | ||
| 401 | } | 398 | } | ||
| 402 | } | 399 | } | ||
| Show All 30 Lines | |||||
| 433 | { | 430 | { | ||
| 434 | auto view = outputView(); | 431 | auto view = outputView(); | ||
| 435 | auto iface = outputViewModel(); | 432 | auto iface = outputViewModel(); | ||
| 436 | if ( ! view || ! iface ) | 433 | if ( ! view || ! iface ) | ||
| 437 | return; | 434 | return; | ||
| 438 | eventuallyDoFocus(); | 435 | eventuallyDoFocus(); | ||
| 439 | 436 | | |||
| 440 | auto index = view->currentIndex(); | 437 | auto index = view->currentIndex(); | ||
| 441 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 438 | auto fvIt = findFilteredView(view); | ||
| 442 | if (QAbstractProxyModel* proxy = m_proxyModels.value(id)) { | 439 | if (fvIt != m_views.end() && fvIt->proxyModel) { | ||
| 440 | auto proxy = fvIt->proxyModel; | ||||
| 443 | if ( index.model() == proxy ) { | 441 | if ( index.model() == proxy ) { | ||
| 444 | // index is from the proxy, map it to the source | 442 | // index is from the proxy, map it to the source | ||
| 445 | index = proxy->mapToSource(index); | 443 | index = proxy->mapToSource(index); | ||
| 446 | } | 444 | } | ||
| 447 | } | 445 | } | ||
| 448 | 446 | | |||
| 449 | QModelIndex newIndex; | 447 | QModelIndex newIndex; | ||
| 450 | switch (selectionMode) { | 448 | switch (selectionMode) { | ||
| ▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Line(s) | 499 | { | |||
| 518 | { | 516 | { | ||
| 519 | if( m_views.isEmpty() ) | 517 | if( m_views.isEmpty() ) | ||
| 520 | { | 518 | { | ||
| 521 | listview = createHelper(); | 519 | listview = createHelper(); | ||
| 522 | 520 | | |||
| 523 | layout()->addWidget( listview ); | 521 | layout()->addWidget( listview ); | ||
| 524 | } else | 522 | } else | ||
| 525 | { | 523 | { | ||
| 526 | listview = m_views.begin().value(); | 524 | listview = m_views.begin().value().view.data(); | ||
| 527 | newView = false; | 525 | newView = false; | ||
| 528 | } | 526 | } | ||
| 529 | } | 527 | } | ||
| 530 | m_views[id] = listview; | 528 | 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 | 529 | | |||
| 532 | changeModel( id ); | 530 | changeModel( id ); | ||
| 533 | changeDelegate( id ); | 531 | changeDelegate( id ); | ||
| 534 | 532 | | |||
| 535 | if (newView) | 533 | if (newView) | ||
| 536 | listview->scrollToBottom(); | 534 | listview->scrollToBottom(); | ||
| 537 | } else | 535 | } else | ||
| 538 | { | 536 | { | ||
| 539 | listview = m_views.value(id); | 537 | listview = m_views.value(id).view.data(); | ||
| 540 | } | 538 | } | ||
| 541 | enableActions(); | 539 | enableActions(); | ||
| 542 | return listview; | 540 | return listview; | ||
| 543 | } | 541 | } | ||
| 544 | 542 | | |||
| 545 | void OutputWidget::raiseOutput(int id) | 543 | void OutputWidget::raiseOutput(int id) | ||
| 546 | { | 544 | { | ||
| 547 | if( m_views.contains(id) ) | 545 | if( m_views.contains(id) ) | ||
| 548 | { | 546 | { | ||
| 547 | auto view = m_views.value(id).view.data(); | ||||
| 549 | if( data->type & KDevelop::IOutputView::MultipleView ) | 548 | if( data->type & KDevelop::IOutputView::MultipleView ) | ||
| 550 | { | 549 | { | ||
| 551 | int idx = m_tabwidget->indexOf( m_views.value(id) ); | 550 | int idx = m_tabwidget->indexOf(view); | ||
| 552 | if( idx >= 0 ) | 551 | if( idx >= 0 ) | ||
| 553 | { | 552 | { | ||
| 554 | m_tabwidget->setCurrentIndex( idx ); | 553 | m_tabwidget->setCurrentIndex( idx ); | ||
| 555 | } | 554 | } | ||
| 556 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | 555 | } else if( data->type & KDevelop::IOutputView::HistoryView ) | ||
| 557 | { | 556 | { | ||
| 558 | int idx = m_stackwidget->indexOf( m_views.value(id) ); | 557 | int idx = m_stackwidget->indexOf(view); | ||
| 559 | if( idx >= 0 ) | 558 | if( idx >= 0 ) | ||
| 560 | { | 559 | { | ||
| 561 | m_stackwidget->setCurrentIndex( idx ); | 560 | m_stackwidget->setCurrentIndex( idx ); | ||
| 562 | } | 561 | } | ||
| 563 | } | 562 | } | ||
| 564 | } | 563 | } | ||
| 565 | enableActions(); | 564 | enableActions(); | ||
| 566 | } | 565 | } | ||
| ▲ Show 20 Lines • Show All 76 Lines • ▼ Show 20 Line(s) | |||||
| 643 | } | 642 | } | ||
| 644 | 643 | | |||
| 645 | void OutputWidget::outputFilter(const QString& filter) | 644 | void OutputWidget::outputFilter(const QString& filter) | ||
| 646 | { | 645 | { | ||
| 647 | QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget()); | 646 | QAbstractItemView *view = qobject_cast<QAbstractItemView*>(currentWidget()); | ||
| 648 | if( !view ) | 647 | if( !view ) | ||
| 649 | return; | 648 | return; | ||
| 650 | 649 | | |||
| 651 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 650 | auto fvIt = findFilteredView(view); | ||
| 652 | auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); | 651 | auto proxyModel = qobject_cast<QSortFilterProxyModel*>(view->model()); | ||
| 653 | if( !proxyModel ) | 652 | if( !proxyModel ) | ||
| 654 | { | 653 | { | ||
| 655 | proxyModel = new QSortFilterProxyModel(view->model()); | 654 | proxyModel = new QSortFilterProxyModel(view->model()); | ||
| 656 | proxyModel->setDynamicSortFilter(true); | 655 | proxyModel->setDynamicSortFilter(true); | ||
| 657 | proxyModel->setSourceModel(view->model()); | 656 | proxyModel->setSourceModel(view->model()); | ||
| 658 | m_proxyModels.insert(id, proxyModel); | 657 | fvIt->proxyModel = QSharedPointer<QSortFilterProxyModel>(proxyModel); | ||
| 659 | view->setModel(proxyModel); | 658 | view->setModel(proxyModel); | ||
| 660 | } | 659 | } | ||
| 661 | QRegExp regExp(filter, Qt::CaseInsensitive); | 660 | QRegExp regExp(filter, Qt::CaseInsensitive); | ||
| 662 | proxyModel->setFilterRegExp(regExp); | 661 | proxyModel->setFilterRegExp(regExp); | ||
| 663 | m_filters[id] = filter; | 662 | fvIt->filter = filter; | ||
| 664 | } | 663 | } | ||
| 665 | 664 | | |||
| 666 | void OutputWidget::updateFilter(int index) | 665 | void OutputWidget::updateFilter(int index) | ||
| 667 | { | 666 | { | ||
| 668 | QWidget *view = (data->type & KDevelop::IOutputView::MultipleView) | 667 | QWidget *view = (data->type & KDevelop::IOutputView::MultipleView) | ||
| 669 | ? m_tabwidget->widget(index) : m_stackwidget->widget(index); | 668 | ? m_tabwidget->widget(index) : m_stackwidget->widget(index); | ||
| 670 | int id = m_views.key(qobject_cast<QTreeView*>(view)); | 669 | auto fvIt = findFilteredView(qobject_cast<QAbstractItemView*>(view)); | ||
| 671 | 670 | | |||
| 672 | if(m_filters.contains(id)) | 671 | if (fvIt != m_views.end() && !fvIt->filter.isEmpty()) | ||
| 673 | { | 672 | { | ||
| 674 | m_filterInput->setText(m_filters[id]); | 673 | m_filterInput->setText(fvIt->filter); | ||
| 675 | } else | 674 | } else | ||
| 676 | { | 675 | { | ||
| 677 | m_filterInput->clear(); | 676 | m_filterInput->clear(); | ||
| 678 | } | 677 | } | ||
| 679 | } | 678 | } | ||
| 680 | 679 | | |||
| 681 | void OutputWidget::setTitle(int outputId, const QString& title) | 680 | void OutputWidget::setTitle(int outputId, const QString& title) | ||
| 682 | { | 681 | { | ||
| 683 | QTreeView* view = m_views.value(outputId, nullptr); | 682 | auto fview = m_views.value(outputId, FilteredView{}); | ||
| 684 | if(view && (data->type & KDevelop::IOutputView::MultipleView)) { | 683 | if (fview.view && (data->type & KDevelop::IOutputView::MultipleView)) { | ||
| 685 | int idx = m_tabwidget->indexOf(view); | 684 | int idx = m_tabwidget->indexOf(fview.view.data()); | ||
| 686 | if (idx >= 0) { | 685 | if (idx >= 0) { | ||
| 687 | m_tabwidget->setTabText(idx, title); | 686 | m_tabwidget->setTabText(idx, title); | ||
| 688 | } | 687 | } | ||
| 689 | } | 688 | } | ||
| 690 | } | 689 | } | ||
| 690 | | ||||
| 691 | QHash<int, OutputWidget::FilteredView>::iterator OutputWidget::findFilteredView(QAbstractItemView *view) | ||||
| 692 | { | ||||
| 693 | for (auto it = m_views.begin(); it != m_views.end(); ++it) { | ||||
| 694 | if (it->view == view) { | ||||
| 695 | return it; | ||||
| 696 | } | ||||
| 697 | } | ||||
| 698 | return m_views.end(); | ||||
| 699 | } | ||||
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*)