[rekonq] Re: Google Code-In: Second Submission

[Benjamin Poulain]

On 12/08/2010 05:15 PM, ext Panagiotis Papadopoulos wrote:
> The second patch from “our” Student Furkan has landed:
> Add a way to copy URLs from the Network Analyzer Panel
>
> http://socghop.appspot.com/gci/task/show/google/gci2010/kde/t129158022937
>
> Is it ok, like it is?

 From a quick look:


It would be nice to sort the includes by alphabetical order (like for 
WebKit). :)


NetworkAnalyzer::contextMenuEvent() mixes spaces and tabs for the 
indentation.


In NetworkAnalyzer::contextMenuEvent(), the parent of the KMenu is 
messing things up. QObjects created on the stack should not have parents.


+	KAction *copy;
+	copy = new KAction(KIcon("edit-copy"),i18n("Copy URL"), this);
Could be:
KAction *const copy = new KAction(KIcon("edit-copy"),i18n("Copy URL"), 
this);
(Just a suggestion, some knows how much I like "const" :)).


In NetworkAnalyzer::contextMenuEvent(), the connect() misses spaces 
after the comas to follow the coding style.


I am not sure how this connection works:
connect( _requestList, SIGNAL(customContextMenuRequested(QPoint)), this, 
SLOT(contextMenuEvent(QContextMenuEvent*))) ;
The arguments seem incompatible to me.

cheers,
Benjamin