Added more features to deletes and breakpoints

Added more features to deletes and breakpoints

[DerekS, 2012-07-16 21:02:14]

didn't have time to finish the review but here are a few thoughts.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:115: if (!fDeletesActivated) {
fDeletesActivated = !fDeletesActivated;

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:125: if(isVisible && fDeletesActivated) {
item->setHidden(isVisible && fDeletesActivated);

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:155: int currentRow = fListWidget.currentRow();
remove this

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:159: fCanvasWidget.drawTo(currentRow);
...drawTo(fListWidget.currentRow());

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.cpp
File debugger/SkDebugCanvas.cpp (right):

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.cpp#newc...
debugger/SkDebugCanvas.cpp:106: int counter = 0;
shouldn't this just be...

SkASSERT(index < commandVector.size());
return commandVector[index]->getVisibility();

[DerekS, 2012-07-17 12:24:35]

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (left):

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#o...
debugger/QT/SkDebuggerGUI.cpp:42: , fToolBar(this)
you also need to initialize your boolean and int member variables here.

, fBreakpointsActivated(false)
, ...

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:175: bool isVisible =
fCanvasWidget.commandIsVisibleAtIndex(currentRow);
remove isVisible

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:178: if (isVisible) {
if (fCanvasWidget.commandIsVisibleAtIndex(currentRow))

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.h
File debugger/SkDebugCanvas.h (right):

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.h#newcode18
debugger/SkDebugCanvas.h:18: #include <vector>
group all the system includes together. System includes are the ones you put <>
around.

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.h#newcode22
debugger/SkDebugCanvas.h:22: bool fFilter;
shouldn't this variable be private?

[chudy, 2012-07-17 13:28:31]

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (left):

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#o...
debugger/QT/SkDebuggerGUI.cpp:42: , fToolBar(this)
On 2012/07/17 12:24:35, DerekS wrote:
> you also need to initialize your boolean and int member variables here.
> 
> , fBreakpointsActivated(false)
> , ...

Done.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp
File debugger/QT/SkDebuggerGUI.cpp (right):

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:115: if (!fDeletesActivated) {
On 2012/07/16 21:02:14, DerekS wrote:
> fDeletesActivated = !fDeletesActivated;

Done. I like this.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:125: if(isVisible && fDeletesActivated) {
On 2012/07/16 21:02:14, DerekS wrote:
> item->setHidden(isVisible && fDeletesActivated);

Done.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:155: int currentRow = fListWidget.currentRow();
On 2012/07/16 21:02:14, DerekS wrote:
> remove this

Done.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:159: fCanvasWidget.drawTo(currentRow);
On 2012/07/16 21:02:14, DerekS wrote:
> ...drawTo(fListWidget.currentRow());

Done.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:175: bool isVisible =
fCanvasWidget.commandIsVisibleAtIndex(currentRow);
On 2012/07/17 12:24:35, DerekS wrote:
> remove isVisible

Done.

https://codereview.appspot.com/6406050/diff/1/debugger/QT/SkDebuggerGUI.cpp#n...
debugger/QT/SkDebuggerGUI.cpp:178: if (isVisible) {
On 2012/07/17 12:24:35, DerekS wrote:
> if (fCanvasWidget.commandIsVisibleAtIndex(currentRow))

Done.

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.cpp
File debugger/SkDebugCanvas.cpp (right):

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.cpp#newc...
debugger/SkDebugCanvas.cpp:106: int counter = 0;
On 2012/07/16 21:02:14, DerekS wrote:
> shouldn't this just be...
> 
> SkASSERT(index < commandVector.size());
> return commandVector[index]->getVisibility();

Done. >_< Also changed all other instances in this class where I use iterating
across the vector instead of accessing the element with an index.

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.h
File debugger/SkDebugCanvas.h (right):

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.h#newcode18
debugger/SkDebugCanvas.h:18: #include <vector>
On 2012/07/17 12:24:35, DerekS wrote:
> group all the system includes together. System includes are the ones you put
<>
> around.

Done. Removed iostream actually since that was only for debugging purposes.

https://codereview.appspot.com/6406050/diff/1/debugger/SkDebugCanvas.h#newcode22
debugger/SkDebugCanvas.h:22: bool fFilter;
On 2012/07/17 12:24:35, DerekS wrote:
> shouldn't this variable be private?

Done.

[DerekS, 2012-07-17 15:09:11]

lgtm