Differential D3548

Add begin/end insert/remove columns to RearrangeColumns
Needs RevisionPublic
Actions

Authored by lepagevalleeemmanuel on Nov 30 2016, 4:14 PM.

Details

Reviewers
dfaure
Group Reviewers
Frameworks
Summary

See https://bugs.kde.org/show_bug.cgi?id=373114

Test Plan

My new application (unpublished) run model tests and add/remove elements in its proxy chain during that. It is how this was discovered.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
lepagevalleeemmanuel updated this revision to Diff 8636.Nov 30 2016, 4:14 PM
lepagevalleeemmanuel retitled this revision from to Add begin/end insert/remove columns to RearrangeColumns.
lepagevalleeemmanuel updated this object.
lepagevalleeemmanuel edited the test plan for this revision. (Show Details)
ltoscano added a reviewer: Frameworks.Dec 28 2016, 8:14 PM
ltoscano set the repository for this revision to R275 KItemModels.
ltoscano added a subscriber: ltoscano.

As a general suggestion, if this fixes the bug, please add the usual line
BUG: xyz
and please add a bit more of explanation in the description so that it's clear even without reading the bug.
(I'm not qualified to comment on the content of the review itself).

kfunk added a subscriber: kfunk.Dec 29 2016, 8:28 AM
kfunk added inline comments.
src/krearrangecolumnsproxymodel.cpp
44

Please no short hand variable names.

cc -> sourceColCount?

dfaure edited edge metadata.Dec 29 2016, 9:02 AM

Can you add a unittest too? This class is fully unittested in krearrangecolumnsproxymodeltest.cpp, it's just that changing the source columns configuration "at runtime" wasn't supported. Just add one more test method, with spies for the signals you emit.

Thanks!

dfaure requested changes to this revision.Apr 15 2020, 7:41 AM

This has been fixed with beginResetModel/endResetModel meanwhile, see D27876.

It's arguable which way is better, because the new set of columns can be really different from the old one....

This revision now requires changes to proceed.Apr 15 2020, 7:41 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 15 2020, 7:41 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

Revision Contents
Changeset List

PathPackages
Msrc/krearrangecolumnsproxymodel.cpp (21 lines)

Diff 8636

View Options

src/krearrangecolumnsproxymodel.cpp

Loading...