Differential D14750

Fix problems with output filters in StandardOutputView plugin
ClosedPublic
Actions

Authored by vkorneev on Aug 11 2018, 9:10 PM.

Details

Reviewers
brauch
kfunk
Group Reviewers
KDevelop
Commits
R32:4fb465589aba: Fix problems with output filters in StandardOutputView plugin
Summary

Multiple problems with filters in StandardOutputView plugin including
discrepancy between output view, filter and proxy model were caused by
using the tab index instead of view index as a key to m_filters and
m_proxyModels maps.

BUG: 343124

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
vkorneev created this revision.Aug 11 2018, 9:10 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptAug 11 2018, 9:10 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
vkorneev requested review of this revision.Aug 11 2018, 9:10 PM
vkorneev added a comment.Aug 11 2018, 9:19 PM

If these changes pass the review, I'd like to also make a little refactoring to this code consisting of using one QMap with structs containing pointers to corresponding view, filter and proxy model instead of 3 QMaps with the same keys in current code. Did I get it right that I should do these changes in separate patch?

vkorneev added a reviewer: brauch.Aug 14 2018, 3:06 PM
vkorneev added a subscriber: brauch.

@brauch , sorry for mentioning you explicitly, just had a feeling that nobody has noticed the request for review :)

kfunk added a subscriber: kfunk.Aug 15 2018, 9:06 AM
In D14750#306789, @vkorneev wrote:

If these changes pass the review, I'd like to also make a little refactoring to this code consisting of using one QMap with structs containing pointers to corresponding view, filter and proxy model instead of 3 QMaps with the same keys in current code. Did I get it right that I should do these changes in separate patch?

Yes, separate patch. And I like your idea of putting this into one map instead! The code as-is is difficult to understand.

vkorneev added a comment.Aug 16 2018, 7:55 AM

Hi @kfunk. Sorry for being a little obtrusive, but I just wanted to clarify this a little. Should I wait for this ad hoc fix to be reviewed and accepted, or should I go directly to the refactoring and fix the bug on the way?

kfunk accepted this revision.Aug 16 2018, 9:30 AM

Just applied this change and tested it: Fixes the mentioned bug. I'll push to 5.2 branch.

Thanks a lot for the patch! Go ahead with refactoring this code in master branch, we'd appreciate it.

This revision is now accepted and ready to land.Aug 16 2018, 9:30 AM
Closed by commit R32:4fb465589aba: Fix problems with output filters in StandardOutputView plugin (authored by vkorneev, committed by kfunk). · Explain WhyAug 16 2018, 9:33 AM
This revision was automatically updated to reflect the committed changes.
vkorneev added a comment.Aug 16 2018, 9:44 AM

Thanks a lot for the patch! Go ahead with refactoring this code in master branch, we'd appreciate it.

Thank you for the review :) I'll proceed to work on this soon.

Revision Contents
Changeset List

Diff 39844

View Options

plugins/standardoutputview/outputwidget.cpp

Loading...