Differential D785

Highlight crashed threads BUG: 283379
ClosedPublic
Actions

Authored by itatriev on Jan 11 2016, 2:00 PM.

Details

Reviewers
mwolff
kfunk
Summary

Provided highlighting crashed threads. Check out related diff - D786.

Test Plan

Tested with simple multi-threaded C++ aplication - works as expected. Here's the code: http://pastebin.com/HxSuVAWB

Diff Detail

Repository
R33 KDevPlatform
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
itatriev updated this revision to Diff 1869.Jan 11 2016, 2:00 PM
itatriev retitled this revision from to Highlight crashed threads BUG: 283379.
itatriev updated this object.
itatriev edited the test plan for this revision. (Show Details)
itatriev added a reviewer: kfunk.
itatriev added a project: KDevelop.
itatriev added subscribers: KDevelop, kdevelop-devel.
itatriev updated this object.Jan 11 2016, 2:04 PM
itatriev edited the test plan for this revision. (Show Details)
itatriev updated this revision to Diff 1872.Jan 11 2016, 2:28 PM

Improved code styling and fixed minor issues(warnings during to compilation).

kfunk edited edge metadata.Jan 11 2016, 3:35 PM

Rest LGTM

debugger/framestack/framestackmodel.cpp
152

Please don't hard-code colors.

Use:

KColorScheme scheme(QPalette::Active);
return scheme.foreground(KColorScheme::NegativeText).color();
debugger/framestack/framestackmodel.h
88

Symmetry: Also add a getter

itatriev updated this revision to Diff 1876.Jan 11 2016, 4:16 PM
itatriev edited edge metadata.

Fixed pointed issues.

itatriev marked 2 inline comments as done.Jan 11 2016, 4:16 PM
mwolff requested changes to this revision.Jan 11 2016, 9:25 PM
mwolff added a reviewer: mwolff.
mwolff added a subscriber: mwolff.
mwolff added inline comments.
debugger/framestack/framestackmodel.h
103

Actually, I'd just make this a bool member of the ThreadItem, which will also simplify the model code where you output the color then. I.e. something like bool crashed; after the int nr;, there's 4 byte of padding there anyways, so why not use it?

This revision now requires changes to proceed.Jan 11 2016, 9:25 PM
itatriev added inline comments.Jan 12 2016, 9:58 AM
debugger/framestack/framestackmodel.h
103

As we've discovered with kfunk, this would be impossilble/difficult(?). He mentioned that model is "lazy-loaded" and I can't use something like "m_threads[index].crashed = . . ." in my changed methods. So, are we okay with current solution?

mwolff accepted this revision.Jan 12 2016, 9:57 PM
mwolff edited edge metadata.

ah ok, I wasn't aware of this. Then it's OK to get this in, imo.

debugger/framestack/framestackmodel.cpp
151

style: remove space after between ( thread.nr.

This revision is now accepted and ready to land.Jan 12 2016, 9:57 PM
itatriev updated this revision to Diff 1911.Jan 13 2016, 1:17 AM
itatriev edited edge metadata.

FIxed code styling.

itatriev marked 3 inline comments as done.Jan 13 2016, 1:18 AM

Revision Contents
Changeset List

Diff 1911

View Options

debugger/framestack/framestackmodel.h

Loading...
View Options

debugger/framestack/framestackmodel.cpp

Loading...